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

[Peter Rabbitson]

On 02/08/2018 03:48 PM, Heinz Ekker wrote:
> Hi!
> 

Hi! Sorry for the delay - my availability is low for the next couple 
weeks. See end of email for a robust workaround.

> 
> The reason I am concerned about this ...

Yes, this is a legitimate problem, thank you for finding and reporting 
it! ( although in the future please consider contacting an author 
directly in private when a potential vulnerability has been identified - 
doing so publicly is somewhat suboptimal )

> 
> It does not replace sanitising user input (which I should do!)

Correct.

> but maybe either the documentation should be changed

Possibly. While the documentation is essentially correct from DBIC's 
perspective, it got invalidated when an unsupervised inexperienced 
contributor added support for function-in-update to SQLA back in [1]

> or the handling of references in store_column.

It's not as much references, it is a semantical clash. I *suspect* ( 
though have not fully thought this through ) that DBICs side should 
unconditionally utilize the -value operator somewhere deep in the 
SQLMaker hand-off. I will have to get back to you on that.

> 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).

This is not a great fix - data_type is essentially free form and is 
never validated: it can be both anything and incorrect.

A solid fix for all of the above ( and potentially similar issues ) 
would be to augment the already-existing injection guard [2] to 
explicitly look for

qr/ \b (?: SELECT | UPDATE | DELETE | INSERT ) \b /ix

I suspect this should go into the default set shipped with SQL::Abstract 
[3] , but have not yet done any testing / analysis of how much impact 
this would have.

As a first step I'd recommend you contact the mojolicious people with 
this workaround, as they currently seem to be the primary driver behind 
SQLA things.

Cheers!

[1] https://github.com/dbsrgits/sql-abstract/commit/0ec3aec7
[2] https://metacpan.org/pod/SQL::Abstract#injection_guard
[3] 
https://metacpan.org/source/ILMARI/SQL-Abstract-1.85/lib/SQL/Abstract.pm#L187-191