[Dbix-class] set_column with references - possible SQL injection

[Heinz Ekker]

Hi!

I recently stumbled over some behaviour that was quite surprising to me: When I pass a hash ref to a column accessor (or set_column), it gets passed on to SQL::Abstract, despite the following in the docs: http://search.cpan.org/~ribasushi/DBIx-Class/lib/DBIx/Class/Row.pm#set_column

> If passed an object or reference as a value, this method will happily attempt to store it, and a later "insert" or "update" will try and stringify/numify as appropriate.

I was expecting something like 'cannot bind a reference' or 'HASH(0x...)' ends up being stored in the column, but instead SQL::Abstract crashed with "SQL::Abstract::puke(): [SQL::Abstract::__ANON__] Fatal: Operator calls in update must be in the form { -op => $arg }"

The reason I am concerned about this is because I am developing a web app that exposes a JSON API. In the update controllers I do something quite naive (but fairly common, I'm afraid), like:

my $rs = $schema->resultset('Object')->find($self->param('id'));
$rs->$_($json->{$_}) for qw/name .../;
$rs->update;

If someone gets the idea to send some creative JSON like this:

{ "whatever": { "-(select password from users where upper(username)='admin')": null } }

SQL::Abstract happily translates this to:

UPDATE table SET whatever=(SELECT PASSWORD FROM USERS WHERE UPPER(USERNAME)='ADMIN') WHERE...

and now I have the admin password in column whatever of a row I have access to, for example. 

As far as I was able to dive down into SQL::Abstract, this might be due to some quirk of handling undef values (so it does not produce "(SELECT...) IS NULL" or something that would break the SQL).

This does not work when doing inserts - here it does what I expect: HASH(0x...) gets stored as value in the column. SQL::Abstract warns: SQL::Abstract::belch(): [SQL::Abstract::__ANON__] Warning: HASH ref as bind value in insert is not supported

It does not replace sanitising user input (which I should do!), but maybe either the documentation should be changed or the handling of references in store_column. My quick fix for my application was to override store_column and die if it gets a reference (for data_types that shouldn't get them).

I have attached a short example using Mojolicious. You can run it with:

perl dbictest.pl get -M POST -H 'Content-Type: text/json' -c '{ "whatever": { "-(select password from users where upper(username)='"'admin'"')": null } }' /todo/hack

Ciao,
Heinz

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dbictest.pl
Type: text/x-perl-script
Size: 2939 bytes
Desc: not available
URL: <http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20180208/8e390d18/attachment.bin>