4
\$\begingroup\$

I've made a Discord Bot in Python that uses the Psycopg module to make calls to a PostgreSQL database in order to display information.

https://github.com/Chombler/cardbot

Specifically, I call this function when a user calls the bot:

def pullCardRecord(recordName):
    success = True
    try:
        print("Trying")
        connection = psycopg2.connect(user = db_credentials[0],
                                        password = db_credentials[1],
                                        host = db_credentials[2],
                                        port = db_credentials[3],
                                        database = db_credentials[4])
        print("connected")
        cursor = connection.cursor()

        join_table_query = '''
        SELECT id
        FROM card
        WHERE card.name = ('%s') ''' % (recordName)

        cursor.execute(join_table_query)
        results = cursor.fetchall()
        print(results)
        if len(results) < 1:
            success = False
            raise ValueError('The name that was given to cardbot didn\'t exist in the card table.')

        join_table_query = '''
        SELECT  name, 
                cardclass.cardclass,
                tribe.tribe, cardtype.cardtype,
                cost, side.side, strength, trait.strengthmodifier, health, trait.healthmodifier,
                trait.trait,
                ability,
                flavor,
                cardset.cardset,
                rarity.rarity
        FROM card
        LEFT JOIN cardtoclass ON card.id = cardtoclass.cardid
        LEFT JOIN cardclass ON cardtoclass.classid = cardclass.id
        LEFT JOIN cardtotrait ON cardtotrait.cardid = card.id
        LEFT JOIN trait ON cardtotrait.traitid = trait.id
        LEFT JOIN cardtotribe ON card.id = cardtotribe.cardid
        LEFT JOIN tribe ON cardtotribe.tribeid = tribe.id
        LEFT JOIN cardtype ON cardtype.id = card.typeid
        LEFT JOIN cardset ON cardset.id = card.setid
        LEFT JOIN rarity ON card.rarityid = rarity.id
        LEFT JOIN side ON card.sideid = side.id
        WHERE card.name = ('%s') ''' % (recordName)

        cursor.execute(join_table_query)
        results = cursor.fetchall()


        print("Printing Results")
        for row in results:
            for col in row:
                print(col)
            print()

        cardInstance = cardObject(results)
        print(cardInstance.information())

        # Print PostgreSQL version
        cursor.execute("SELECT version();")
        record = cursor.fetchone()
        print("You are connected to - ", record,"\n")

    except (Exception, psycopg2.Error) as error :
        print ("Error retrieving card information using PostgreSQL,", error)
    finally:
        #closing database connection.
        if(connection):
            cursor.close()
            connection.close()
            print("PostgreSQL connection is closed")
        return(cardInstance.information() if success else "I'm sorry, I couldn't find a card with that name.")

I want to make sure that the bot is protected from any direct sql injections before I deploy it. Is it currently fairly safe or do I need to add anything to protect its integrity?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Unpacking

Try this rather than hard-coded indexing:

    user, password, host, port, database = db_credentials
    connection = psycopg2.connect(user, ...)

Or, if you're able to rearrange those parameters such that they match those of the connect signature:

  • dbname – the database name (database is a deprecated alias)
  • user – user name used to authenticate
  • password – password used to authenticate
  • host – database host address (defaults to UNIX socket if not provided)
  • port – connection port number (defaults to 5432 if not provided)

then you can simply do

connection = psycopg2.connect(*db_credentials)

Logs

Consider replacing

    print("Trying")
    print("connected")

with calls to the actual logging module, which is more configurable and maintainable.

Quote escapes

'The name that was given to cardbot didn\'t exist in the card table.'

can be

"The name that was given to cardbot didn't exist in the card table."

In-application queries

Your join_table_query is long. There are several approaches to improve this - either save a view in the database (my favourite), or a stored procedure (common but I think it's overkill in this case).

Injection

the bot is protected from any direct sql injections before I deploy it

This is directly vulnerable:

'''... WHERE card.name = ('%s') ''' % (recordName)

Never (ever) use string formatting to insert parameters to a query. All DB connection libraries have anticipated this concern and most approach it using "prepared statements".

This article is old but relevant. The actual reference shows that you should be passing vars as a sequence or a mapping, which will prevent injection.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.