How to untaint system call in CGI.pm

Question

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?

Karlise

Answer 1

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.

Kirkland

Answer 2

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

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

Viewable

+1. Why use system commands when there's a better way to achieve the desired result. – Counterfoil

Answer 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.

Counterfoil

Or do some matching like: my ($untaint_data) = $td =~ m/^\s*(\w+)\s*$/;. – Flyn

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. – Selfexamination

Answer 4

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!

Homeopathist

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." – Toothless