DBIx::Class field name (identifier) injections

[duelafn]

In DBIx::Class case insensitve substring search., chrestomanci encounters a problem using LOWER(me.name) as a field name while using the quote_char (or perhaps newer quote_names) option. This inspired me to dig into escaping of identifiers (in particular, protection against injection) in DBIx::Class.

Consider a simple table "hash" with two text columns, "key" and "value". Simple code to set up an SQLite table and populate it with three rows below.

NOTE: unlinks and creates file called "test.sqlite" in current directory.

package MyApp::Schema::Result::Hash;
use base qw/DBIx::Class::Core/;

__PACKAGE__->table('hash');
__PACKAGE__->add_columns(qw/ key value /);
__PACKAGE__->set_primary_key('key');

1;

package MyApp::Schema;
use base qw/DBIx::Class::Schema/;

__PACKAGE__->register_class(Hash => "MyApp::Schema::Result::Hash");

1;


package main;
use warnings; use strict; use 5.010;
use DBI;

my $DB_FILE = "test.sqlite";
unlink $DB_FILE;

my @DSN = ("dbi:SQLite:dbname=$DB_FILE","","", { AutoCommit => 1, Rais
+eError => 1 });
DBI->connect(@DSN)->do($_) for split /\n/, <<'INIT';
CREATE TABLE hash (key text, value text);
INSERT INTO hash VALUES ('foo', 'FOO');
INSERT INTO hash VALUES ('bar', 'BAR');
INSERT INTO hash VALUES ('baz', 123);
INIT

We can see that, by default, DBIx::Class does not protect against SQL injection in identifiers:

my $schema = MyApp::Schema->connect(@DSN);
my $hash   = $schema->resultset("Hash");

my $good_key = "key";
say $hash->search({ $good_key => "foo" })->count;
##  executes: SELECT COUNT( * ) FROM hash me WHERE ( key = ? ): 'foo'
##  says 1

my $evil_key = "1 = 1 ) --";
say $hash->search({ $evil_key => "foo" })->count;
##  executes: SELECT COUNT( * ) FROM hash me WHERE ( 1 = 1 ) -- = ? ):
+ 'foo'
##  says 3

SQL::Abstract and DBIx::Class have a quote_char option (newer DBIx::Class recommends quote_names instead). These options will cause identifiers to be quoted. The intention is to protect against column names which may be keywords in SQL.

$schema->storage->sql_maker->quote_char('"');
say $hash->search({ $evil_key => "foo" })->count;
##  executes: SELECT COUNT( * ) FROM "hash" "me" WHERE ( "1 = 1 ) --" 
+= ? ): 'foo'
##  says 0

The quote_*, however, do not provide injection protection (at least to my standards) as they simply perform the naïve quoting.

my $evil_key2 = 'key" = "key" ) -- ';
say $hash->search({ $evil_key2 => "foo" })->count;
##  executes: SELECT COUNT( * ) FROM "hash" "me" WHERE ( "key" = "key"
+ ) -- " = ? ): 'foo'
##  says 3

Proper quoting for injection protection can only (as far as I can tell) be performed via quote_identifier on the DBI storage object

$schema->storage->sql_maker->quote_char(undef);
my $Q_evil_key2 = $schema->storage->dbh->quote_identifier($evil_key2);
say $hash->search({ $Q_evil_key2 => "foo" })->count;
##  executes: SELECT COUNT( * ) FROM hash me WHERE ( "key"" = ""key"" 
+) -- " = ? ): 'foo'
##  says 0

Is it true that there is no fully automated injection protection for identifiers in SQL::Abstract / DBIx::Class? Would it be reasonable to post a bug / wishlist item against SQL::Abstract and DBIx::Class requeesting a quote_identifiers option or some such (to be used like quote_names) that performs the more robust quoting? Am I dumb and just simply missing something?

[frew]

Ok, this isn't really the right place to bring this up at all (we have a mailing list.) But, the short answer is you are correct, nothing more is done than the naive quoting. I would recommend posting an RT, but realize that to do what you are asking for correctly requires the long awaited SQLA2, as every part of the query would need to be aware of what columns are in what table. So, again, please post an RT, but don't hold your breath.