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

#!I:\Interwoven\TeamSite\iw-perl\bin\iwperl -Tw use strict; use warnings; use diagnostics; use CGI; use CGI::Carp qw(fatalsToBrowser warningsToBrowser set_message); set_message("It's not a bug, it's a feature!"); $CGI::POST_MAX = 1024 * 100; # maximum upload filesize is 100K ######################## Build the form ######################### my $q = CGI->new; my $OUTFILE; my @ts_dir = qw(); my $video = 0; print $q->header; warningsToBrowser(1); print $q->start_html(-title => "Upload file to web server"), $q->h3('Import File'), $q->start_multipart_form( -name => 'main_form', -ENCTYPE => 'multipart/form-data', ), 'Click on the browse button to choose a filename: <BR>', $q->filefield( -name => 'filename', -size => 75, -maxlength => 80, ), $q->hr, $q->submit(-value => 'Upload file'), $q->hr, $q->end_form; ########## Look for uploads that exceed $CGI::POST_MAX ########## if (!$q->param('filename') && $q->cgi_error()) { print $q->cgi_error(); print <<'EOT'; <p> The file you are attempting to upload exceeds the maximum allowable fi +le size. <p> Please refer to your system administrator EOT print $q->hr, $q->end_html; exit 0; } ###################### Upload the file ######################## if ($q->param) { save_file($q); } print $q->end_html; exit 0; #------------------------------------------------------------- sub save_file { my ($q) = @_; my ($bytesread, $buffer); my $num_bytes = 1024; my $totalbytes; my $source_filename = $q->upload('filename'); my $untainted_filename; my $filename_orig = $source_filename; my $dir_path = ""; my $destination_filename = ""; if (!$source_filename) { print $q->p('You must enter a filename before you can uplo +ad it'); return; } ################# CGI Variables to get path and user id ########## +###### print "<pre>\n"; my $user_id = ""; foreach my $key (sort keys(%ENV)) { if ($key =~ /QUERY_STRING/) { push(@ts_dir,$ENV{$key}); #print "$ENV{$key}<p>"; } elsif ($key =~ /IWUSER/) { $user_id = $ENV{$key}; #print "$user_id<p>"; } } print "</pre>\n"; foreach my $d_path (@ts_dir) { $d_path =~ s/&done.*//; $d_path =~ s/.*=\/\/testserver//; $dir_path = $d_path; #print "Destination Directory:<BR>"; #print "$dir_path<p>"; } # original vpath my $orig_dir_path = $dir_path; $dir_path =~ s/\//\\/g; $dir_path = "Y:"."$dir_path"."\\"; # absolute directory path my $ts_dir_path = $dir_path; ################ Untaint $source_filename ################# if ($source_filename =~ /\.swf/ || $source_filename =~ /high/ || $ +source_filename =~ /low/) { $video = 1; $source_filename =~ /(\w+.\w+)$/; $untainted_filename = $1; } else { $video = 0; $source_filename =~ /(\w+.\w+)$/; $untainted_filename = $1; } # restore original vpath $dir_path = $orig_dir_path; if ($video == 0 && $dir_path =~ /\/design\/user\/videos/) { #print "dir_path: $dir_path\n"; die <<"EOT"; Only '.swf' files and files with 'high' or 'low' in the name will be u +ploaded to /design/user/videos. EOT } elsif ($video == 1 && $dir_path !~ /\/design\/user\/videos/) { #print "dir_path: $dir_path\n"; die <<"EOT"; Video files not allowed to be uploaded to current destination folder. EOT } if ($untainted_filename =~ m/\.\./) { die <<"EOT"; Your upload filename may not contain the sequence '..' Rename your file so that it does not include the sequence '..', and tr +y again. EOT } opendir(DIR, $ts_dir_path); my @files = grep(/\.*$/,readdir(DIR)); closedir(DIR); $source_filename = $filename_orig; $source_filename =~ s|.*\\||; # determine if the file already exists on server foreach my $ts_file (@files) { if ($ts_file =~ m|$source_filename|) { die <<"EOT"; Your upload filename already exists at folder location on server. Rename your file, and try again. EOT } } # End of foreach my $ts_file (@files) { $dir_path = $orig_dir_path; if ($video == 1 && $dir_path =~ /\/design\/user\/videos/) { $destination_filename = "Y:\\main\\$untainted_filename"; } elsif ($video == 0 && $dir_path !~ /\/design\/user\/videos/) { $dir_path =~ s/\//\\/g; $dir_path = "Y:"."$dir_path"."\\"; print "dir_path: $dir_path<BR>"; $destination_filename = "$dir_path"."$untainted_filename"; print "destination_filename: $destination_filename<BR>"; } $source_filename = $filename_orig; print "<u>Upload Started:</u> <BR>$source_filename <BR><i>uploadin +g to</i> <BR>$destination_filename<BR>"; # If running this on a non-Unix/non-Linux/non-MacOS platform, be s +ure to # set binmode on the $OUTFILE filehandle, refer to # perldoc -f open # and # perldoc -f binmode open ($OUTFILE, '>', $destination_filename) || die "Couldn't open +$destination_filename for writing: $!"; binmode($OUTFILE); while ($bytesread = read($source_filename, $buffer, $num_bytes)) { $totalbytes += $bytesread; print $OUTFILE $buffer; } die "Read failure" unless defined($bytesread); unless (defined($totalbytes)) { print "<p>Error: Could not read file ${untainted_filename}, "; print "or the file was zero length."; } else { print "<p><u>Upload Completed:</u> <BR>$source_filename <BR><i +>uploaded to</i> <BR>$destination_filename ($totalbytes bytes)"; } close $OUTFILE or die "Couldn't close $destination_filename: $!"; } #-------------------------------------------------------------

Hi Guys, I cant seem to get around this error below and I know why but not sure how to resolve it:

Insecure dependency in open while running with -T switch at I:/Interwoven/TeamSite/httpd/iw-bin/custom_import.cgi line 188.

The issue I suspect is with the untainted file I'm trying to manipulate to upload file from other directories in lines 171-176. I'm able to upload file in lines 168-169 but not the opposite.

Any help is greatly appreciated, Thanks

Replies are listed 'Best First'.
Re: Insecure dependency in open while running with -T switch
by riverron (Sexton) on Sep 21, 2011 at 02:13 UTC
    It seems like you still have tainted data lying around.
    
    Try untainting the $dir_path within your 'else' block in lines 171-176.
    
    You might want to check the Laundering-and-Detecting-Tainted-Data from perlsec as well.
    
    Hope this helps.
    
Re: Insecure dependency in open while running with -T switch
by Anonymous Monk on Sep 21, 2011 at 10:22 UTC

    The issue I suspect is with the untainted file I'm trying to manipulate to upload file from other directories in lines 171-176. I'm able to upload file in lines 168-169 but not the opposite.

    The error message tells you the issue is $destination_filename is tainted

    Looking how you build it, I am horrified to see

    my $user_id = ""; foreach my $key (sort keys(%ENV)) { if ($key =~ /QUERY_STRING/) { push(@ts_dir,$ENV{$key}); #print "$ENV{$key}<p>"; } elsif ($key =~ /IWUSER/) { $user_id = $ENV{$key}; #print "$user_id<p>"; } }

    That is the craziest thing I've seen in a long time :)

    Part of your confusion stems from skipping perlintro, so

    Drop what you're doing and read perlintro (or Tutorials) please!

    Then replace that loop with

    my @ts_dir = $ENV{QUERY_STRING}; my $user_id = $ENV{IWUSER};

    The other part is you're not quite sure what it should do, so it does too much

    You should document your subroutines
    What input the subroutines takes
    what output it produces
    what effects it has / what actions it performs (copy file)

    After you're done with perlintro, read http://search.cpan.org/dist/CGI.pm/lib/CGI.pm#Accessing_the_temp_files_directly

    And reduce save_file to this

    #!I:\Interwoven\TeamSite\iw-perl\bin\iwperl -T -- use strict; use warnings; use diagnostics; use CGI; use CGI::Carp qw(fatalsToBrowser warningsToBrowser set_message); use File::Copy qw/ copy /; Main( @ARGV ); exit( 0 ); sub Main { set_message("It's not a bug, it's a feature!"); local $CGI::POST_MAX = 1024 * 100; # maximum upload filesize is 1 +00K my $q = CGI->new; print $q->header; warningsToBrowser(1); print buildForm( $q ); if ( !$q->param('filename') && $q->cgi_error() ) { ... } else { save_file($q); } print $q->end_html; } sub save_file { my( $q ) = @_; my $filename = $query->param('uploaded_file'); my $destination_filename = File::Spec->catfile( $hardcoded_destina +tion_directory, WashFilename( $filename ) ); copy( $filename->handle, $destination_filename ) or die "Copy to +( $destination_filename) failed: $!"; }

    WashFilename should create a new (normalized) filename, not warn about dots and other things (action instead of talk)

    Something like this (save as WashFilename-test.pl)

    You would benefit greatly from reading http://learn.perl.org/books/beginning-perl/

    And maybe Modern Perl: the free book

    That is all I have time for, it only took ~3 hours :)

      Thanks guys, appreciate the help/advice.

Re: Insecure dependency in open while running with -T switch
by Anonymous Monk on Sep 21, 2011 at 09:20 UTC

    To quote davido

    Please make sure when you cross-post that you identify such, so that people here (or there) don't end up working on a problem that is solved. Identifying cross-posting will promote collaborative progress too. Getting a bunch of people doing research for you in parallel without knowing about each others' progress defeats the collaboration aspect of public forums.

    Cross posted at http://forums.devshed.com/perl-programming-6/upload-file-issue-848441.html