views:

255

answers:

2

I'm developing a relatively small application to talk to PostgreSQL, and wanted to get some feedback on how far is too far to go with regards to protecting against SQL injection.

The application is in Perl and does not use any ORM modules (just DBI). The SQL statements are constructed in the typical fashion with placeholders:

my $sql = "SELECT * FROM $cfg->{tablename} WHERE foo = ?";
my $sth = $dbh->prepare($sql);
$sth->execute('bar');

The reason the tablename is interpolated is that the application has to perform the same operation against multiple tables, all of which have a column 'foo'.

Using the ? placeholder protects against most of the simple SQL injection attacks. My question is around the tablename, which you can't use a placeholder for. The table comes from the config file, but the app supports a --configfile switch to use an alternate config file.

The database credentials are stored in the config file. So if an attacker could craft a config file (or replace the default one) with one in which $cfg->{tablename} was replaced with something malicious, then the application could be "tricked" into running the malicious code.

For an attacker to do this, they'd have to have valid database credentials already, otherwise the application wouldn't connect. If they have credentials, then they can just craft their own code using DBI or use the psql cli to do something malicious.

I see two possible ways to protect against this:

  • switch to an ORM, in which case I'd be doing something on the order of $orm->get_class_for_table($cfg->{tablename}
  • use a regex to sanitize the tablename before preparing the SQL statement
  • use $dbh->quote_identifier()

Obviously the second way is the "cheap and cheerful" one. But given the statement about credentials above, are either of these approaches really warranted? How much effort is too much when it comes to forcing the attacker to just use an alternate attack vector (as opposed to effort that actually prevents the attack?

+7  A: 

use quote_identifier:

my $sql = "SELECT * FROM ".
  $dbh->quote_identifier($cfg->{tablename}).
  " WHERE foo = ?";
Alexandr Ciornii
I updated the question to reflect this as a possibility (it's another way to do the regex-style sanitization).My real question remains though: if the attacker is in a position where quote_identifier will save you, they're in a position to go do the same thing via psql.
James F
Of course it will not save you from attacker that knows DB login and pass. But it creates more fool-proof apps. And in case that DB login data is stored elsewhere, it will help from attacks.Between, quote_identifier() does not delete symbols from table names, it just escapes symbols like quotes and surround name with quotes that correspond to your DB.
Alexandr Ciornii
One interesting detail: quote_identifier does not work for Oracle.
Massa
+1  A: 

In these cases, I make my own version of placeholders, and I check the values I would insert as the table names to ensure it's a table name that I expect. That means more than just checking that the table exists; some operations should only affect certain tables but not others.

I wouldn't use a regex to sanitize names. Either they get it right and it's in your list of allowed names, or they got it wrong and you give them an error.

brian d foy