Cookie Monsters in the Closet

Warning from the General Perl Surgeon:

Googling for Perl Apache handlers can be harmful to your mental health. Really.

Yesterday I came across the following code fragment within an Apache handler:

my $id = undef;
foreach (keys %cookies)
   if(($_) eq 'T2G')
      $id = $cookies{$_}->value;

Aren't the brackets formatted so nicely? God (and hopefully also the programmer) only knows why this is not just a simple

my $id = $cookies{T2G} && $cookies{T2G}->value;

It would also test whether the entry for T2G existed, and if so, would assign the value to $id. Sounds so easy when I say it, doesn't it? And it also works nicely if the hash is bigger.

But maybe God (or the programmer) was paid by LoCs? Or maybe he (or He) was a PHP programmer? Any excuse will do at this point.

Further down there was also

if((index($str, "bed") >= 0) || (index($str, "Bed") >= 0))    
    return Apache2::Const::DECLINED;

This effectively shouts into your face "Hey, look what cool IF-statements a PHP programmer can write." But it also screams "Hey, do not bother me with regular expressions.". So maybe I have no point in saying that

return Apache2::Const::DECLINED if $str =~ /[Bb]ed/;

is shorter and more readable. And maybe faster too.

Alright, you might object, this is a PHP programmer, so he does not know Perl too well. You say, "Robert, give him a break."

No, I won't. Because I continue to read:

my $db = DBI->connect("",
                      "erwin", "ochcocbo") || die("$DBI::errstr");

Which tells me a lot more about the guy. And his database (username and password modified to protect the incompetent). But the 'fun' does not stop here. The next line continues with

my $sth = $db->prepare("select count(*)
                        from login
                        where sessionID=\"$id\""

Textbook SQL injection scenario. I just love to see someone tweak $id into

xxx'' ; DELETE FROM login; --

But to continue on the Perl track, it then also escapes me why someone would bother to write

@row = $sth->fetchrow_array;

if($row[0] eq 1)
    return 1;
    return 0;

instead of just writing

return $row[0] eq 1;

Well, unless you are a Bond University graduate.

It is 'not only' 8 times shorter, it also exposes much better an unnecessary dependency on how values are actually stored in the relational database. The programmer is relying on a boolean value 1, but the obvious intent is to test whether there is at least one row in the database.

So maybe a simple

return @row;

would have sufficed. And in the same breath we would have gotten rid of the suspiciously suspicious eq 1 which string-compares numbers!

Then I continue to read:

if (processAuthorization($str,$args,$row2[0]) eq 0)
   goto deny;
return Apache2::Const::DECLINED;

   return Apache2::Const::DECLINED;

which is borderline absurd. Not sure on which side of the border, though.

First, this is the first time I have seen a goto in an Apache handler. But there always must be a first time for everything. Hopefully there is also a last time for everything.

Then, again, that eq 0 is just sooooo wrong.

But, thirdly, would the above not be written a bit more concisely as

    unless processAuthorization($str,$args,$row2[0]);
return Apache2::Const::DECLINED;

And now I even understand what it is supposed to do. Well, almost.

There are many more fishy things. Finally, it is left as exercise for the reader to reduce the following madness to 2-3 lines real Perl:

$pos =     index($location, '/data/');
if($pos > -1)
   $pos += 6;
   $location = substr($location, $pos);
   if(index($location, '.txt') > -1)
      $pos = index($location, '_Overlap');
      $location = substr($location, 0, $pos);
      $args = $location;
   elsif(($pos = index($location, '.tsv')) > -1)
      $location = substr($location, 0, $pos);
      $tmp = $pos - 1;
      while($tmp > 0)
         if(($pos = index($location, 'p', $tmp)) > -1)
            $args = substr($location, 0, $tmp);
         elsif(($pos = index($location, 'chr', $tmp)) > -1)
            $args = substr($location, 0, $tmp);
         - - $tmp;

I now have my theories why this handler was named CookieMonster. Please let me believe that there is a hell for programmers. And that it is me sitting at the gate.

Ah, and before I forget: There is also a documentation inside the handler:


Proper operation requires proper configuration of
Apache and specific knowledge of the session/access rules,
as well as mod_perl itself. Refer to the proper

I could not have said it more properly.

Happy Christmas.

Posted In


In assuming only best intentions, he or He or she was only practicing for ... why not in a different language to gain more ideas.

Marcus Meisel (not verified) | Sun, 12/23/2007 - 13:21

Re: obfuscation

As Dr. House uses to say:

Intentions are useless. What people do, matters.

And in an obfuscated program, it is maybe not overly wise to hard-code a password.

So, nice theory, but I do not believe in best intentions.

Everybody lies.
rho | Sun, 12/23/2007 - 13:50

if whatever then true else false

This construction (using if to return true/false) I see quite a lot. It seems many people have difficulties with the idea that boolean expressions are, well, expressions that return boolean values, and not some special thing you write in if/while statements. I've never figured out why, but I see (and fix) this a lot.

Lars Marius Garshol (not verified) | Sun, 12/23/2007 - 16:10

Re: if whatever then true else false

I've never figured out why, but I see (and fix) this a lot.

I assume that 'fixing' has to do with the program, not with the programmer? For me, this is just a temporary fix, because this programmer may produce more.

I am more interested in methods involving physical measures, such as electroshocks or strong psycho-pharmaca. Maybe even brain surgery. See a lot of this on the Dr. House TV show.

It always works there.

rho | Sun, 12/23/2007 - 16:24