Re: Bug in Class::DBI::Untaint

[prev] [thread] [next] [Date index for 2004/04/23]

On Fri, Apr 23, 2004 at 01:07:52PM +0200, Thomas Klausner wrote:
> Hi!
> 
> I found some buggy behaviour of Class::DBI::Untaint (running with CDBI
> 0.96_07), or maybe CGI::Untaint works a little bit different from what I'm
> expecting..
> 
> I set up a class and do something like:
> ___PACKAGE__->columns(All => qw/itemid orders/);
> ___PACKAGE__->constrain_column(orders => Untaint => "integer");
> 
> I later cannot set orders to 0 because:
>    validate_column_values error: orders My::Order orders fails 'untaint'
>    constraint with '0' 
> 
> The same happens with printable fields and an empty string.
> 
> I've attached a modified t/01.t from the CDBI::Untaint distribution that
> shows this behaviour.
> 
> Unfortunatly, I now next to nothing about CDBI constraints or the
> CGI::Untaint internals, so I couldn't provide a patch. If this is a bug and
> not a mistake by me, BTW.
> 
> If it is a bug, I could take a look at solving it, if somebody (Tony?) gives
> me a short hint on where to look..

It looks like this is caused by the fact that add_constraint() will add a
trigger that will _croak() if the return value of _constrain_by_untaint
is 'false' - which it will be if the input if 'false'.

I don't really know how CGI::Untaint works but it seems to return undef
on a value not matching the regex so perhaps changing _constrain_by_untaint
in Class::DBI::Untaint to pass back whether the result of extract() was defined
rather than the value itself would solve the problem, i.e. :

  defined CGI::Untaint->new({ $col => +shift })->extract("-as_$type" => $col);

instead of just :

  CGI::Untaint->new({ $col => +shift })->extract("-as_$type" => $col);

I don't know enough about whether this will still untaint it properly
but it looks like the problem is in that area anyhow.

Colm