3

I have a Perl script which opens a file, processes it and prints some output. The input file is gzipped.

the path to the $file is passed to the script as an arugment.

Below is the current solution I'm using:

open(my $fh, "-|", "$gzcat $file") or die("Cannot open $file$!");

The script has failed in Checkmarx's security audit recently, with the following error:

<script> gets user input for the $fh element. This element’s value then flows through the code without being properly sanitized or validated and is eventually displayed to the user in method <method>. This may enable a CrossSite-Scripting attack.

I have tried validating the file exists with perl -f, and also removing unwanted characters using $file =~ s/[^A-Za-z0-9_\-\.\/]//g;, yet it does not satisfy Checkmarx.

I would like to know what is the proper way of sanitzing an input which contains a path to a file in Perl.

Emil Gelman
  • 125
  • 1
  • 10
  • 1
    Sounds like you're in urgent need of [proper shell escaping](https://stackoverflow.com/questions/3212128/perl-equivalent-of-phps-escapeshellarg). **Never, ever** dump user-supplied input into a shell argument without proper escaping. – tadman Jan 31 '20 at 18:58
  • @tadman The input is supplied by a different application running on the server that generates the file name. – Emil Gelman Jan 31 '20 at 19:53
  • 2
    You still can't trust that `$file` is harmless, or devoid of shell characters. For example `$file = "a space.txt"` will not work. – tadman Jan 31 '20 at 19:54

2 Answers2

1

As long as you are on Perl 5.8 or newer on an OS that supports forking, or 5.22 or newer on Windows, you can use the list form of pipe open to bypass the shell when running your command. This avoids problems where the filename contains metacharacters the shell will interpret, such as & and spaces.

open(my $fh, "-|", $gzcat, $file) or die("Cannot open $file: $!");

However, this is not validation or sanitization as requested, but it is important to avoid both vulnerabilities and misbehavior. The cross-site scripting possibility that is mentioned would be due to the filename being displayed as mentioned later; if it is displayed in an HTML page for example, you must HTML-escape it, most templating systems have methods to do this.

Grinnz
  • 9,093
  • 11
  • 18
  • The filename itself is not being printed, only the content of the file is. HTML-escaping the file path would mess up the path itself, for example the slashes in the path which would get translated to %2F – Emil Gelman Jan 31 '20 at 19:55
1

I ended up removing unwanted characters with

$file =~ s/[^A-Za-z0-9_\-\.\/]//g;

Checking that the file exists with Perl -f, and opening the file using IO::Uncompress::Gunzip.

This passes Checkmarx's audit.

Emil Gelman
  • 125
  • 1
  • 10
  • 1
    I cover this in [Mastering Perl](https://www.masteringperl.org)'s "Secure Programming Techniques" chapter. You can taint the data coming in from a filehandle, and do close to what you did to untaint it. Another way to say "removing unwanted" is to say "keeping wanted"; the difference between what Mark Jason Dominus calls the American and Prussian approaches. – brian d foy Feb 06 '20 at 12:52