in reply to Re: Testing file input validity in FF
in thread Testing file input validity in FF

When uploading the file, the file isn't saved, but the data within the file is processed. Here's the code for processing the uploaded file (I've cleaned it up and simplified. It's a program I inherited.): 
sub upload {
        my $self = shift;
        my $q = $self->query;
        if(untaintFile($q->param('file')) && $q->upload('file')) {
                my $filename=untaintFile($q->param('file'));
                ($filename) = $filename =~ /([[:ascii:]]+)/;
                my $file = $q->upload('file');
                my $data;
                while(<$file>) {
                        $data .= $_;
                }
                if($data) {
                        my $thisfile = File->parse($data);
                        
                        if(my $format_error = $thisfile->check_formatt
+ing()) {
                                ...warn the user if there are any file
+ format errors...
                                return;
                        }
                        my $file_id = $self->file_upload({name => $fil
+ename, data => $data});
                }
        }

        $self->header_type('redirect');
        $self->header_props(-url => "$CONFIG{userbase}/user.pl");
        return;
}

sub parse {
        my $proto = shift;
        my $origfile = shift;
        my @lines = ();
        my $file = $origfile;
        LINE: while(length($file) >= 94) {
                $file =~ s/^\D*//;
                push @lines, substr($file, 0, 94, '');
                if(@lines && substr($lines[-1], 0, 1) eq '9') {
                        last LINE; # Don't need to parse any more afte
+r the file control record
                }
        }

        my @batches = ();
        for(my $i = 0; $i < @lines; $i++) {
                if(substr($lines[$i], 0, 1) eq '5') { # Batch Header
                        my @tempbatch = ();
                        for(; $i < @lines; $i++) {
                                push @tempbatch, $lines[$i];
                                if(substr($lines[$i], 0, 1) eq '8') { 
+# Batch Control
                                        last;
                                }
                        }
                        push @batches, File::Batches->new(\@tempbatch)
+;
                }
        }

        my $self = {
                file            => $origfile,
                lines           => \@lines,
                batches         => \@batches,
                total_subtract     => substr($lines[-1], 31, 12),
                total_add    => substr($lines[-1], 43, 12),
        };

        bless $self, $proto;
        return $self;
}
sub file_upload {
        my $self = shift;

        my %params = validate(@_, {
                name    => { type => SCALAR },
                data    => { type => SCALAR },
        });

        my $thisFile = File->parse($params{data});

        my @effective_dates;
        tie @effective_dates, 'Array::Unique';

        @effective_dates = $thisFile->get_effective_dates;

        ...insert data into database...

        return $id; #return the primary key
}

[download]

Replies are listed 'Best First'.
Re^3: Testing file input validity in FF
by Anonymous Monk on Feb 18, 2009 at 08:28 UTC
    No browser will send a full path for input type file.
[reply]
      IE does...
[reply]
        Its not supposed to do that.
[reply]
Re^3: Testing file input validity in FF
by scorpio17 (Canon) on Feb 18, 2009 at 15:00 UTC

    Here's a few checks you could easily add:

    • Was a file name entered?
    • Does this file contain data?
    • Is the data of the expected size/type/format?

    It looks like you're assuming that it's a text file, and you read it into memory line by line. But what if someone accidentally sends you a large binary file (maybe it's the correct file, only it's been zipped)?

    Here's a revised version of your upload() function that uses read() to read the file into a buffer 1kb at a time, and abort if it looks too big (you can set your own limit). 

    sub upload {
  my $self = shift;
  my $q = $self->query;
  if(untaintFile($q->param('file')) && $q->upload('file')) {
    my $filename=untaintFile($q->param('file'));
    ($filename) = $filename =~ /([[:ascii:]]+)/;

    unless($filename) {
      croak "no filename given!";
    }

    my $file = $q->upload('file');

    unless($file) {
      croak "can't get filehandle for upload!";
    }

    my $data;
    my $buffer;
    my $bytesread = 0;
    my $totalbytes = 0;

    while($bytesread = read($file, $buffer, 1024)) {
      $totalbytes += $bytesread;
      if($totalbytes > $MAX_DATA_LIMIT) {
        croak "upload file too big!";
      }
      $data .= $buffer;
    }

    unless ($data) {
      croak "no data!";
    } else {
      my $thisfile = File->parse($data);
                        
      if(my $format_error = $thisfile->check_formatting()) {
        # warn if any format errors found
      return;
                        }
      my $file_id = $self->file_upload({
        name => $filename, 
        data => $data,
      });
    }
  }
  $self->header_type('redirect');
  $self->header_props(-url => "$CONFIG{userbase}/user.pl");
  return;
}

    [download]
[reply]
[d/l]
      Thanks for the input. I'll try that. What about taint? Will that satisfy taint?
[reply]

In Section Seekers of Perl Wisdom