fileno, taint and CGI.pm

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

I stumbled over a nasty bug in CGI.pm today. The kind of bug that surprises me not only because it's there, but because I never stumbled over it sooner. Turns out if you try to pass a '-' (dash) as a URL parameter value, and your script is running with the 'T' (taint) flag, then CGI->new dies with:

Insecure dependency in require while running with -T switch at /usr/lib/perl5/vendor_perl/5.10.0/CGI.pm line 516

This happens in version 3.33 (which I'm running) and doesn't appear to be corrected in the latest 3.42 version. A bug report was posted Mar 25 of this year, so at least I'm not the first to hit it. The problem boils down to this particular section of code:

496: sub init {
...
516:  for my $fh (grep {defined(fileno($_))} @$val) {
517:    seek($fh,0,0); # reset the filehandle.
518:  }
[download]

The init() function walks through all query parameters and attempts to reset the file handle for each file name matching the parameter value. The parameter values are obviously tainted as they come from outside the script so using them as input to fileno() already has a funny smell to it. The init() function is called by the new() function, so near as I can tell, this only affects you if you if you use the object interface to CGI (my $obj = CGI->new). Maybe that's one reason why more people aren't complaining?

But CGI doesn't die on all URL values, just '-'. I did a little more digging and sure enough fileno() is called with every URL value, but only '-' caused an insecure dependancy compaint. So I wrote up a script so I could test further:

#!/usr/bin/perl -T

use strict;
use warnings;

# snipped directly from CGI.pm
# $val comes from (tained) URL parameters this this case
# I'm using command line input to provide tainted test input

my $val = \@ARGV;

for my $fh (grep {defined(fileno($_))} @$val) {
  seek($fh,0,0); # reset the filehandle.
}
[download]

Go ahead, try it yourself. I can pass all kinds of tainted values to fileno(), and none of them are flagged, except '-'.

So my observations are:

This affects you if you use the CGI object interface

ALL url parameter values are passed into fileno() tainted

This code executes in the CGI init() method (called from CGI->new), so you have no control over validating input before CGI processes it

My solution is to pass a value into new, "my $obj = CGI->new('')", which is enough to trigger a different logic path which doesn't involve reseting file handles (it doesn't matter that the parameter passed into new is empty, just that it's defined).

But I feel there are questions that need answering here:

Why does fileno() flag '-' as tainted and not other values? Surely it's not testing the values before testing for taintness?

Is there a scenario where CGI could mess with the application file handles (or at least die again) because of particular values in the URL?

This strikes me needless code execution at best, and as a major security risk at worse. How has it been allowed to slip by without anyone noticing?

Comment on fileno, taint and CGI.pm Select or Download Code

Replies are listed 'Best First'.
Re: fileno, taint and CGI.pm by Anonymous Monk on Mar 29, 2009 at 04:57 UTC
# Why does fileno() flag '-' as tainted and not other values? Surely it's not testing the values before testing for taintness? I don't think so, but I didn't look too hard :) http://perl5.git.perl.org/perl.git?a=search&h=HEAD&st=grep&s=fileno Is there a scenario where CGI could mess with the application file handles (or at least die again) because of particular values in the URL? No. This strikes me needless code execution at best, and as a major security risk at worse. How has it been allowed to slip by without anyone noticing? Congratulations, you're the first(?) to notice :) Its not exactly needless (lets just say CGI is complicated), and its not really a security risk, but you could argue it is a bug in perl-5.10.0 C:\>perl -e "warn 1;warn fileno shift" - 1 at -e line 1. Warning: something's wrong at -e line 1. C:\>perl -Te "warn 1;warn fileno shift" - 1 at -e line 1. Insecure dependency in require while running with -T switch at -e line + 1. BEGIN failed--compilation aborted. C:\>more - die 666 C:\>perl -e"require '-' 666 at - line 1. Compilation failed in require at -e line 1. C:\>perl -e"require shift" - 666 at - line 1. Compilation failed in require at -e line 1. C:\>perl -Te"require shift" - Insecure dependency in require while running with -T switch at -e line + 1. C:\> [download] You can see the taint error message is total nonsense (there is no require being performed). I get no error with ActivePerl-5.8.9.825 , ActivePerl-5.8.4.810. A good alternative to CGI is CGI::Simple	[reply] [d/l]
Re^2: fileno, taint and CGI.pm by Anonymous Monk on Mar 29, 2009 at 05:07 UTC
Under the debugger it doesn't get triggered C:\>perl -d -Te"warn 1; warn fileno shift" - Loading DB routines from perl5db.pl version 1.3 Editor support available. Enter h or `h h' for help, or `perldoc perldebug' for more help. main::(-e:1): warn 1; warn fileno shift DB<1> n 1 at -e line 1. at -e line 1 main::(-e:1): warn 1; warn fileno shift DB<1> n Warning: something's wrong at -e line 1. at -e line 1 Debugged program terminated. Use q to quit or R to restart, use o inhibit_exit to avoid stopping after program termination, h q, h R or h o to get additional info. DB<1> n Use `q' to quit or `R' to restart. `h q' for details. DB<1> Use `q' to quit or `R' to restart. `h q' for details. DB<1> q C:\> [download]	[reply] [d/l]
Re^2: fileno, taint and CGI.pm by ruzam (Curate) on Mar 29, 2009 at 17:31 UTC
but you could argue it is a bug in perl-5.10.0 A yes, Perl 5.10 that explains a few things. Perl 5.8 doesn't complain about an insecure dependency (just checked). I find it difficult to believe that of all the web applications in the world written with CGI.pm using new(), there isn't more chatter over breaking apps with a simple '-' URL value. But I can believe that the majority of those same apps are probably running under Perl 5.8, and maybe not even under taint mode at that. Still, I'm dumbstruck by CGI's behaviour to use the parameter 'value' in the first place. Sure CGI is complicated, and I don't confess to understand what's going on here. This particular piece of code appears to be related to preserving parameter values between calls (a feature of CGI I've never needed). But what's the point of referencing a filename based on the parameter value?	[reply]
Re^3: fileno, taint and CGI.pm by Anonymous Monk on Mar 29, 2009 at 19:57 UTC
I knew you were going to ask :) `$filename = param('uploaded_file'); ...` [download] temporary file that CGI.pm creates during upload spooling (see below). The filename returned is also a file handle. You can read the contents of the file using standard Perl file reading... CGI::Simple avoids such overloading	[reply] [d/l]
Re^2: fileno, taint and CGI.pm by ruzam (Curate) on Mar 30, 2009 at 20:43 UTC
Dang! My CGI->new('') trick has a show stopper side effect of preventing form parameters from being passed into the app (although it doesn't seem to get in the way of URL parameters). Now I'll have to find another way to get around the '-' input problem. CGI::Simple looks like a great alternative, and switching may be an option later, but for now I've got to get CGI to work out of the box.	[reply]