I hate the param method from CGI

Scalar and list content is a nice and advanced feature of perl. Sometimes I think it's a bit too smart for us who use perl.

In our code we have a lot of method calls like this

$obj->foo( name1 => $value, name2 => bar() );

We do a lot of web stuff and often we like to pass the user input to a method like this:

$obj->foo( name1 => $value, name2 => $cgi->param("inputkey") );

This code is bad! It should be

$obj->foo( name1 => $value, name2 => scalar($cgi->param("inputkey")) );

This is because the call to param is in list content. The bug is nasty because it often has security implications. The user can give multiple parameters to the web-script and then overwrite the parameters to the foo method.

This is an example:

$obj->foo( is_superuser =>0, name => $query->param("name") );

The user is able to call foo in superuser mode if he calls the script with the querystring

?name=Anders&name=is_superuser&name=1

So who is too blame for this mess? The programmer? Well we have some very bright people and they made this mistake many times in the past. When many people make the same mistake many times it could be argued that it isn't only their fault. Should we blame perl? Maybe the context feature is just too advanced. We could however also blame the CGI module for having a crappy interface. Or blame the perl community for allowing CGI to be such an important module.

6 Comments

I blame the programmers for not properly scrubbing their inputs. You should have something at the boundary between the outside world and your code that unpacks, cleans, and validates all the incoming CGI parameters and part of that scrubbing would be checking that you're not getting more "name" parameters than you expect.

CGI supports multiple values for a given parameter (as it should), the CGI and apreq "param()" behavior seems perfectly sensible to me given how the CG interface is defined.

Sorry, but there are absolutely situations where CGI's param method is problematic. For example, say you have a field "alias" where the user can add as many aliases as they want. So there can be 0 or more values. Sure there are hacks around this (e.g. calling them alias1 - aliasN,) but we can't always control such things.

Plack solves this with Hash::MultiValue where it always returns a the last value unless you explicitly request the full list with 'get_all'.

Who cares?

Why does it matter who to blame? Say you decide that it's Lincoln Stein. Now what?

Blame is worthless.

Leedo already commented but Hash::MultiValue is invented to solve this exact issue. More background: http://bulknews.typepad.com/blog/2009/12/perl-why-parameters-sucks-and-what-we-can-do.html

I find it a little odd that you a working directly on the CGI object. However, a little method that gives you a scalar whatever may have been posted, wouldn't have hurt.

Leave a comment

About anielsen

user-pic I blog about Perl.