Catalyst best practices: don’t allow GETs for login

Saturday, 15 December 2007

There is a wonderful annual series of articles on Catalyst this time of year called the Catalyst Advent Calendar. Day 5 contained this bit of code.

Authenticating your user

sub login : Local {
     my ( $self, $c ) = @_;

     if ($c->authenticate
              ( { 
                  username => $c->request->params->{'username'},
                  password => $c->request->params->{'password'}
              } ))
     {
        # $c->authenticate returns a true value if authentication succeeds
        # so display the login successful page here.
     } else {
        # or undef is authentication failed.  
        # so display the 'try again' page here.
     }
 }

While it’s a good article, the snippet has a security problem, albeit a minor one. You should never expose private data in the URI. Even if you write your login form as POST, that code, as is, will accept submissions as GET.

It means that someone lazy and unconcerned with security could set a bookmark to login with a URI like–

http://your-site.com/app/login?username=doofus;password=guessme

Then if they click off the site from that page or that page has any externally loaded resources at all—images, scripts, ads, polls, metrics—their account info has just been broadcast and recorded somewhere else.

It’s easy to prevent that sort of problem though. Do not accept the GET parameters. They come from the query string portion of the URI. Instead only accept the body parameters. They come with POST requests.

Catalyst, eminently flexible, offers you access to either or both at once through the request object. GET with query_params, POST with body_params, and both through params.

my $username = $c->request->body_params->{username};
my $password = $c->request->body_params->{password};

$c->authenticate({username => $username,
                  password => $password
                 })

Best practice would probably be to never use params (the method or the hash key) but always use query_params or body_params depending on whether the concerned form’s method is GET or POST.

Update (10 October 2008): something to keep in mind in this case is that Catalyst::Plugin::Unicode will not decode parameters accessed this way. So–

my $i18n_username =
  Encode::decode("utf-8", $c->request->body_params->{username});
digg stumbleupon del.icio.us reddit Fark Technorati Faves
Your information (required) Name*
Email*
Website

* Indicates required fields; email is used for validation and is not displayed on the site.

Your comment
Commenting on Catalyst best practices: don’t allow GETs for login
Title

Body is limited to ≈1,000 words. Paragraphs—but not line breaks—are automatically inserted. Valid XHTML is required. These are the allowed tags–

<a href=""></a> <br/> <acronym title=""></acronym> <abbr title=""></abbr> <code></code> <pre></pre> <tt></tt> <ins></ins> <del></del> <hr/> <cite></cite> <b></b> <i></i> <sup></sup> <sub></sub> <strong></strong> <em></em> <h1></h1> <h2></h2> <h3></h3> <q></q> <blockquote></blockquote>