5

I have the following CGI script:

#!/usr/bin/perl -T
use strict;
use warnings;
use CGI::Carp qw(fatalsToBrowser);
use CGI qw/:standard/;
my $query = CGI->new();
my $searchterm = param('name');

my $file = "justafile.txt";
# Begin searching terms and ignoring case
my @entries = `grep -i \"$searchterm\" $file`; # Line10
chomp @entries;
# Do something

When I execute the command it gives me this

Insecure dependency in `` while running with -T switch at /foo/cgi-bin/mycode.cgi line 10.

How line 10 can be fixed?

neversaint
  • 50,277
  • 118
  • 274
  • 437

4 Answers4

8

The whole point of tainting is to ensure that unchecked input cannot be supplied to potentially unsafe functions.

In this case, your $searchterm variable might contain unexpected input that might allow an attacker to execute arbitrary programs on your system.

Hence, you either need to:

  1. untaint the variable by ensuring that it matches a pre-determined regexp (see @flesk's answer), at which point Perl assumes that you know what you're doing, or

  2. don't use backticks (per @eugene y's answer).

If you're using backticks you should also specify the full path to the grep command so that you're not depending on $PATH.

Alnitak
  • 313,276
  • 69
  • 379
  • 466
5

Use the built-in grep function, e.g.:

open my $fh, '<', $file or die $!;    
my @entries = grep /$searchterm/i, <$fh>;
Eugene Yarmash
  • 119,667
  • 33
  • 277
  • 336
3

The -T switch only warns you about possible tainted input: http://perldoc.perl.org/perlsec.html#Taint-mode

You need to untaint it yourself, for example using

my $safe_searchterm = "";
$safe_searchterm .= $_ for $searchterm =~ /\w+/g;

That's not a very sophisticated test though, and possibly not too safe either, unless you have complete control over what \w matches.

EDIT: Changed my minimal solution to reflect the info given in the comments below.

flesk
  • 7,046
  • 3
  • 19
  • 33
  • 1
    Or do some matching like: my ($untaint_data) = $td =~ m/^\s*(\w+)\s*$/;. – XoR Nov 22 '11 at 15:43
  • 4
    Substitution does not untaint a value. Only assignment from a captured regular expression group (`($untainted) = $tainted =~ /(pattern)/;`, `$tainted =~ /(pattern)/; $untainted=$1;`) will untaint an expression. See "Laundering and Detecting Tainted Data" in [`perlsec`](http://search.cpan.org/perldoc?perlsec). – mob Nov 22 '11 at 18:58
3

I think the problem here is that the backtick operator is effectively executing code outside of the perl environment, and is therefore quite rightly not trusted, ie. tainted.

You could, of course, try doing something like this before the offending line:

$ENV{"PATH"} = "";

You will probably still get an error from this line:

my $file = "justafile.txt";

To fix that, you could probably just give it an absolute path, eg:

my $file = "/home/blah/justafile.txt";

You would almost certainly have to give an absolute path to the grep command you are executing with the backtick operator too, as clearing the environment variables will loose the path. In other words do:

# Begin searching terms and ignoring case
my @entries = `/bin/grep -i \"$searchterm\" $file`; # Line10

You might also want to copy the value of $ENV before you clear it, just in case you need it later...

Hope some of this helps!

Eugene Yarmash
  • 119,667
  • 33
  • 277
  • 336
  • 2
    Conceptual error: The data are tainted, not the operation done on them. See the output of: `perl -T -MDevel::Peek -e'Dump \@ARGV' "I solemnly swear that I'm up to no good."` – daxim Nov 22 '11 at 15:24