CODEWITHSUNDEEP
perlcgitaint
neversaint
neversaint

Reputation: 64004

How to untaint system call in CGI.pm

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?

Upvotes: 5

Views: 1603

Answers (4)

flesk
flesk

Reputation: 7579

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.

Upvotes: 3

Neil S Hamilton
Neil S Hamilton

Reputation: 51

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!

Upvotes: 3

Alnitak
Alnitak

Reputation: 339816

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.

Upvotes: 8

Eugene Yarmash
Eugene Yarmash

Reputation: 149806

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

open my $fh, '<', $file or die $!;    
my @entries = grep /$searchterm/i, <$fh>;

Upvotes: 5

Related Questions