Re: CERT Advisory CA-97.25 - CGI_metachar

Greg Bacon (gbacon@ADTRAN.COM)
Tue, 11 Nov 1997 11:09:38 -0600

In message <Pine.SUN.3.94.971110165908.19839B@dfw.dfw.net>,
Aleph One quoted:
: #!/usr/cert/bin/perl
: $_ = $user_data = $ENV{'QUERY_STRING'}; # Get the data
: print "$user_data\n";
: $OK_CHARS='a-zA-Z0-9_\-\.@'; # A restrictive list, which
: # should be modified to match
: # an appropriate RFC, for example.
: eval "tr/[$OK_CHARS]/_/c";
: $user_data = $_;
: print "$user_data\n";
: exit(0);

This code allows more than what's in $OK_CHARS (or at least what was
intended to be in $OK_CHARS) to pass, namely '\', '[', and ']'.

Misunderstandings by the author of the above:

o The only character a backslash quotes inside single quotes is a
single quote. The code above results in $OK_CHARS containing
characters that weren't intended to be there.

o The transliteration operator (tr///) takes lists of characters as
arguments (think of them like ordered character classes). Square
brackets aren't special in tr///.

o The eval EXPR form can be very slow since what's in EXPR must be
compiled and run like a little program.

Perl has excellent documentation. Each of these points is covered in
the perlop(1) and perlfunc(1) manpages.

Something like this would be better

#! /usr/bin/perl

$_ = $user_data = $ENV{'QUERY_STRING'};
print "$user_data\n";

$OK_CHARS = '-a-zA-Z0-9_.@'; # leading dash is regular

s/[^$OK_CHARS]/_/go; # assumes $OK_CHARS never changes

$user_data = $_;

print "$user_data\n";

Perl also has a feature known as tainting by which data from the outside
world may not be used unexamined in operations that affect the outside
world. These and other security features are outlined in the perlsec(1)
manpage. Good example CGI code (such as Randal's WebTechniques columns)
always enables tainting.

I'm concerned when I see basic misunderstandings and flawed code in a
security advisory.

Greg