Mailing List Archive: Catalyst: Users

Potential query string pollution vulnerability?

Index | Next | Previous | View Threaded

tobias.kremer at gmail

Jun 16, 2009, 2:11 AM

Post #1 of 8 (1226 views)
Permalink

Potential query string pollution vulnerability?

Hi all,

I just experienced a nasty case of query string pollution
vulnerability in one of my Catalyst/DBIC apps. I think that the
circumstances under which this applies are not _that_ rare, so I
figured it'd be best to inform the world.

Imagine the following code in one of your actions:

sub crashme :Local {
my( $self, $c ) = @_;
my $result = [. $c->model( 'Foo' )->search( {
-or => [
name => $c->req->param( 'name' )
],
} ) ];
}

To me, this never looked like a potential security threat because
$c->req->param('name') is correctly inserted/quoted via bind
parameters, right? Well, let's see what happens, if we "pollute" the
query string a bit:

/crashme?name=Foo&name=Bar

This results in the following SQL:
SELECT ... FROM ... me WHERE ( ( name = ? OR Bar IS NULL ) )

Oh oh! :(

'Bar' is used as a column name here because Catalyst::Request::param
returns an Array if the caller desires it (wantarray). Solving this
problem is easy: Either force scalar context, or force array context
and take only the first element.

I'm not sure if it makes sense or is even possible to fix this within
DBIC and/or Catalyst. By the way, I'm using DBIC 0.08107 and Catalyst
5.71.

What do you think?

--Tobias

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

fireartist at gmail

Jun 16, 2009, 2:19 AM

Post #2 of 8 (1158 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

2009/6/16 Tobias Kremer <tobias.kremer [at] gmail>:
> Hi all,
>
> I just experienced a nasty case of query string pollution
> vulnerability in one of my Catalyst/DBIC apps. I think that the
> circumstances under which this applies are not _that_ rare, so I
> figured it'd be best to inform the world.
>
> Imagine the following code in one of your actions:
>
> sub crashme :Local {
> my( $self, $c ) = @_;
> my $result = [. $c->model( 'Foo' )->search( {
> -or => [.
> name => $c->req->param( 'name' )
> ],
> } ) ];
> }
>
> To me, this never looked like a potential security threat because
> $c->req->param('name') is correctly inserted/quoted via bind
> parameters, right? Well, let's see what happens, if we "pollute" the
> query string a bit:
>
> /crashme?name=Foo&name=Bar
>
> This results in the following SQL:
> SELECT ... FROM ... me WHERE ( ( name = ? OR Bar IS NULL ) )
>
> Oh oh! :(
>
> 'Bar' is used as a column name here because Catalyst::Request::param
> returns an Array if the caller desires it (wantarray). Solving this
> problem is easy: Either force scalar context, or force array context
> and take only the first element.
>
> I'm not sure if it makes sense or is even possible to fix this within
> DBIC and/or Catalyst. By the way, I'm using DBIC 0.08107 and Catalyst
> 5.71.
>
> What do you think?

I remember someone pointing this out at YAPC-europe 2006 - so I'd be
surprised if it's not mentioned in the docs somewhere.
Personally, using HTML-FormFu I add the SingleValue constraint to all
form fields, by default.
I would expect all form validation modules to provide a similar
functionality - in which case, never touch $c->req->params() and
always use $form->params().

Carl

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

onken at houseofdesign

Jun 16, 2009, 2:36 AM

Post #3 of 8 (1156 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

Am 16.06.2009 um 11:11 schrieb Tobias Kremer:

> Hi all,
>
> I just experienced a nasty case of query string pollution
> vulnerability in one of my Catalyst/DBIC apps. I think that the
> circumstances under which this applies are not _that_ rare, so I
> figured it'd be best to inform the world.
>
> Imagine the following code in one of your actions:
>
> sub crashme :Local {
> my( $self, $c ) = @_;
> my $result = [. $c->model( 'Foo' )->search( {
> -or => [
> name => $c->req->param( 'name' )
> ],
> } ) ];
> }
>
> To me, this never looked like a potential security threat because
> $c->req->param('name') is correctly inserted/quoted via bind
> parameters, right? Well, let's see what happens, if we "pollute" the
> query string a bit:
>
> /crashme?name=Foo&name=Bar
>
> This results in the following SQL:
> SELECT ... FROM ... me WHERE ( ( name = ? OR Bar IS NULL ) )
>
> Oh oh! :(
>
> 'Bar' is used as a column name here because Catalyst::Request::param
> returns an Array if the caller desires it (wantarray). Solving this
> problem is easy: Either force scalar context, or force array context
> and take only the first element.
>
> I'm not sure if it makes sense or is even possible to fix this within
> DBIC and/or Catalyst. By the way, I'm using DBIC 0.08107 and Catalyst
> 5.71.
>
> What do you think?

You are not validating your input. That's all there is to say...

moritz

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

tobias.kremer at gmail

Jun 16, 2009, 2:52 AM

Post #4 of 8 (1156 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

> You are not validating your input. That's all there is to say...

True, but I think that many people are led to believe that their input
is being correctly quoted by DBIC which in most cases it is, but in
this particular case it is not. I'm just trying to safe people from
the consequences of this rare but not so obvious behaviour. Sorry, if
this is not what this mailing list is about. Geez ...

--Tobias

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

orasnita at gmail

Jun 16, 2009, 4:14 AM

Post #5 of 8 (1153 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

From: "Tobias Kremer" <tobias.kremer [at] gmail>
> Hi all,
>
> I just experienced a nasty case of query string pollution
> vulnerability in one of my Catalyst/DBIC apps. I think that the
> circumstances under which this applies are not _that_ rare, so I
> figured it'd be best to inform the world.
>
> Imagine the following code in one of your actions:
>
> sub crashme :Local {
> my( $self, $c ) = @_;
> my $result = [ $c->model( 'Foo' )->search( {
> -or => [
> name => $c->req->param( 'name' )

Try:

name => $c->req->params->{name}

I think this was the recommended way, exactly for the reason you described.

Octavian

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

arodland at comcast

Jun 16, 2009, 4:19 AM

Post #6 of 8 (1162 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

On Tuesday 16 June 2009 04:11:19 am Tobias Kremer wrote:
> To me, this never looked like a potential security threat because
> $c->req->param('name') is correctly inserted/quoted via bind
> parameters, right? Well, let's see what happens, if we "pollute" the
> query string a bit:
>
> /crashme?name=Foo&name=Bar
>
Using $c->req->param for this kind of purpose (or, if you ask certain people,
for any purpose) is discouraged, and has been discouraged as long as I can
remember, for this reason. Use $c->req->params and validate your input.
(Incidentally, if you'd used $c->req->params->{name} the behavior you would
have gotten would have been "WHERE name='Foo' OR name='Bar'" which can be a
really useful behavior straight out of the box -- but the point stands that
you have to know what your data is, know what your data needs to be, and make
sure that the two are reconcileable before you do anything :)

Andrew

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

tobias.kremer at gmail

Jun 16, 2009, 4:35 AM

Post #7 of 8 (1164 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

On Tue, Jun 16, 2009 at 1:14 PM, Octavian Rasnita<orasnita [at] gmail> wrote:
> Try
> name => $c->req->params->{name}
> I think this was the recommended way, exactly for the reason you described.

Thanks a lot! I didn't know that this was the recommended practice.

Apparently, TIMTOWTDI striked again! :(

--Tobias

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

bobtfish at bobtfish

Jun 16, 2009, 6:03 AM

Post #8 of 8 (1162 views)
Permalink

Re: Potential query string pollution vulnerability? [In reply to]

Tobias Kremer wrote:
> Thanks a lot! I didn't know that this was the recommended practice.
>
> Apparently, TIMTOWTDI striked again! :(

The docs on Catalyst::Request::param don't help to make this (and the
possible consequences of using this method) clear.

If someone would like to volunteer to write a paragraph making this
crystal clear, and recommending the other methods of accessing the
parameters, then this would be _very welcome_.

Cheers
t0m

_______________________________________________
List: Catalyst [at] lists
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst [at] lists/
Dev site: http://dev.catalyst.perl.org/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads