Re(2) (ichimunki): Security issues when allowing file upload via CGI

[dmmiller2k]

Roger that!!

Had it not been for Micro$oft's feature-laden behemoths, (and their commensurate security patches, and security-patch patches, and so on, new ones of which seem to be required almost daily), nevermind OS-related issues, there might never have erupted as pervasive an anti-virus cottage industry as we have (which has since become a full-fledged industry).

I think you should quarantine uploaded files and run the shell command,

file {upload filename}

on them to confirm they are what they purport to be.

use File::Basename;

sub validate_image _file {
  my $fn = shift;
  my %file_types = ( jpg => 'JPEG file',
                     jpeg => 'JPEG file',
                     gif => 'GIF file, v8[79]' );

  my $ext = lc (fileparse $fn )[-1];  # get suffix
  return 0 unless exists $file_types{$ext};

  my $file_cmd_output = `file $fn`;
  chomp $file_cmd_output;
  return 0 unless $file_cmd_output =~ /^$file_types{$ext}$/;

  # OK, we probably have what we think we have
  # go ahead and make it accessible, etc.
  accept_file( $fn );  # ... or whatever
  return 1;
}

Update: The expectation here is that before this subroutine is called, a file has already been uploaded (ostensibly one whose name ends in .jpg, .jpeg, or .gif) and "quarantined" -- that is, stored somewhere "safe", out of harm's way -- and that the parameter, $fn, to the sub is the full path to this file.

(Thanks, nufsaid, for bringing up the issue of the tainted-ness of $fn)

dmm

Just call me the Anti-Gates ...

[nufsaid]

I like the idea, but don't you have to be careful of this line?

my $file_cmd_output = `file $fn`;

$fn is tainted and doesn't this give them the chance to sneak a command in via $fn? Need to make sure $fn is clean.

Joe.

[dmmiller2k]

Sigh.

Yes, you're right about $fn being tainted, but I was speaking more to the principle of using the standard *NIX command, "file" (or reasonable facsimile, such as the perl module File::MMagic, for example) to check the type of the file by examining the first several bytes (or more).

Certainly, by the time I intended this code to run, a file would have been uploaded to some directory somewhere. My expectation was that $fn would, at the time of the call, contain the full pathname to this file, and would not necessarily have been entered directly by a web-page user.

But then, I DID fail to document these expectations (read: preconditions, using programming-by-contract terminology), so I deserve to be chastised for it.

(takes 40 lashes with a wet noodle)

dmm

You can give a man a fish and feed him for a day ...
Or, you can teach him to fish and feed him for a lifetime