CodeSOD: Brillant Perls

Many years ago, a Paula Bean type was hired to make a Perl-based website. It became the company's flagship product, at least briefly, until a better version of the product was ready. But early adopters adopted it, and thus it had to keep operating, because you can't throw a way a 800kLOC web application just because it's fragile and unmaintainable.

And then the site got hacked. So now, fixing everything becomes incredibly important, and the task fell to Erik. He needed to do a security audit and identify vulnerabilities. Alone. In a 800kLOC application of extremely questionable code quality. For bonus challenges, there is no testing environment available and no budget to stand one up- even if anyone knew exactly what actually needs to be in that environment, because there's a bunch of databases and packages and extra software and no one is entirely sure what the production environment is.

Erik started by opening the first file he saw and giving it a skim. It was a small one, with only about two hundred lines in it. The first line was:

$ENV{DOCUMENT_ROOT} = $ENV{DOCUMENT_ROOT};

That wasn't an auspicious start, but Perl is a finicky beast. "Maybe," Erik thought, "this was a workaround for an interpreter bug, or a line that once made sense but no longer does after refactoring."

Things went downhill quickly, though.

$pwd = $1000001 + int rand(1 + 1999999 - 1000001);

What is this? Is $pwd a password? Not based on how it's used in the rest of the code. What is the variable $1000001 (this is Perl, we know it's a scalar variable because it starts with a $)? Is it even defined? Or was it a typo and that $ shouldn't be there?

What was true was that it wasn't a security hole, so Erik didn't touch the line and kept moving on.

As he scrolled, he hit a point where the syntax highlighter just gave up and started spamming out nonsense. It wasn't hard to track it back to this line:

$html .= "Click below for" more information.<br>";

A spurious quote in the middle of the string. Based on the revision history, that change was added three years earlier. So this script had been broken and failing due to syntax errors for three years- but it still kept getting invoked. Fortunately, the interpreter kept failing to compile it, so it never did anything.

Erik marked the file as "free from security vulnerabilities" based on "it doesn't do anything", and moved onto the next file. 200 lines down, only 799,800 more to go.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!