Discord bot with Psycopg

Question

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?

Answer 1

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.