http://qs321.pair.com?node_id=98905

tachyon has asked for the wisdom of the Perl Monks concerning the following question:

If you wish to allow users to enter a search term and use Perl's internal grep function to search an index file is this script a secure way to do it? By using quotemeta on the user input I can not see how you can hack this, even though the untainting of the user search term is global to allow searching for strings other than pure alphanumberic. I am worried about the interpolation into the grep. Can anyone see security holes? 

#!/usr/bin/perl -wT
use strict;
use CGI;
use Fcntl (':flock');

# clean up the environment for CGI use
BEGIN {
    delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};
    $ENV{'PATH'} = '/bin:/usr/bin:';
}

$CGI::DISABLE_UPLOADS = 1;
$CGI::POST_MAX        = 1024;
my $query             = new CGI;
my $db_file           = 'c:/test.pl';
my $flock             = 1;
my $timeout           = 15;

my $find = $query->param('search');
$find    = quotemeta $find;
$find    = ($find =~ m/^(.+)$/) ? $1 : '';
die_nice("Please specify a search term!") unless $find;

# get the data into an array, retrun a reference to that array
my $data_ref = get_data($db_file);

# do the search
my @lines = grep{ /$find/i }@$data_ref;

# do something with @lines

[download]

cheers

tachyon

s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

Replies are listed 'Best First'.
(Ovid) Re: Is this CGI search secure?
by Ovid (Cardinal) on Jul 23, 2001 at 08:00 UTC

    I can't see anything in your script that is insecure. However, You don't provide us with the code for get_data() and you don't show us what you do with @lines.

    The only user-supplied data appears to be $find and with your setting $CGI::POST_MAX to 1K, it looks perfectly safe. However, what do you do with @lines? Since they are going to match what you have in user-supplied data, there could potentially be issues there.

    Is this just a test script? I noticed that $db_file appears to be a perl program and that doesn't seem to quite match the variable. Does &get_data do anything with $find?

    Cheers,
    Ovid

    Vote for paco!

    Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

[reply]

      Hi Ovid,

      I just supplied the bit I am worried about to keep it as short as possible and was just grepping a convenient perl file on my system. Yes to proof of concept. The @ lines data just get munged and goes back to the browser with links to the found stuff so that is/should be fine.

      The get_data() routine is just your usual bread and butter Perl. As the user does not interact directly with it I did not think it necessary to be inclued. Here it is anyway. 

      sub get_data {
    my $file = shift;
    open (FILE, "<$file") or die_nice("Oops can't read $file: $!\n");
    if ($flock) {
        my $count = 0;
        until (flock FILE, LOCK_SH) {
            sleep 1;
            die_nice("Can't lock file '$file': $!\n") if ++$count >= $
+timeout;
        }
    }
    my @file = <FILE>;
    close FILE;
  return \@file;
}

      [download]

      My main worry was null byte; "\n..."; "/@file; `rm rf`; #" type hacks. These won't work but are there others? Oh the die_nice() prints the usual "Sorry the system can not respond to your request due to routine maintenence, please try again later." back to the browser and sends the admin the real message ;-)

      cheers

      tachyon

      s&&rsenoyhcatreve&&&s&n.+t&"$'$`$\"$\&"&ee&&y&srve&&d&&print

[reply]
[d/l]
        Not that you are going to worry about this too much if the search input is only reused on a page returned to that very user, but you may want to do something to escape any HTML that is included in the user input before it gets sent back to the browser (in an HTML document). At best, it will goof up the display (i.e. the browser will interpret the tags as tags). At worst, if one user is allowed to enter input that will be output to another user there is significant potential for foul play (javascript, pictures of Barney... people do weird stuff if this hole is open).
[reply]
(MeowChow) Re: Is this CGI search secure?
by MeowChow (Vicar) on Jul 23, 2001 at 11:13 UTC
    I don't see any security issues either, but I would recommend for the sake of efficiency that you place a /o modifier on your regex, unless you plan to run this under mod_perl.
      
    my @lines = grep /\Q$find/io, @$data_ref;

    [download]
    You can also skip all the quotemeta and untainting rigamorole for $find, if using the above construct. Perl doesn't care about the taintedness of a variable interpolated into a regex, unless the var contains an eval-group and use re 'eval' is on, which taint mode will summarily ignore. 
       MeowChow                                   
               s aamecha.s a..a\u$&owag.print
[reply]
[d/l]

      You might want to a look at this apachecon talk to see how you can safely use the /o modifier under mod_perl. Basically saying:

      eval q{my @lines = grep /\Q$find/io, @$data_ref;}

      would solve the /o modifier problem.

      b.t.w. this talk addresses many other good points that come in handy when programming for mod_perl

[reply]
[d/l]
        You're better off using the qr operator rather than evaling run-time code for this sort of thing (I'm surprised this wasn't mentioned in the ApacheCon talk):
          
        my $re = qr/\Q$find/i;
my @lines = grep /$re/, @$data_ref;

        [download]
           MeowChow                                   
               s aamecha.s a..a\u$&owag.print
[reply]
[d/l]

Back to Seekers of Perl Wisdom