Re: CERT Advisory CA-97.25 - CGI_metachar

[gbacon () ADTRAN COM (Greg Bacon)]

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