views:

40

answers:

1

Hi Everyone,

I have a bit of a mystery here that I am not quite understanding the root cause of. I am getting an 'Insecure dependency in unlink while running with -T switch' when trying to invoke unlink from a script. That is not the mystery, as I realize that this means Perl is saying I am trying to use tainted data. The mystery is that this data was previously untainted in another script that saved it to disk without any problems.

Here's how it goes... The first script creates a binary file name using the following

# For the binary file upload
my $extensioncheck = '';

my $safe_filename_characters = "a-zA-Z0-9_.";
  if ( $item_photo )  
  { 
    # Allowable File Type Check
    my ( $name, $path, $extension ) = fileparse ( $item_photo, '\..*' );
    $extensioncheck = lc($extension);
    if (( $extensioncheck ne ".jpg" ) && ( $extensioncheck ne ".jpeg" ) &&
        ( $extensioncheck ne ".png" ) && ( $extensioncheck ne ".gif" ))
    {
      die "Your photo file is in a prohibited file format.";  
    }

    # Rename file to Ad ID for adphoto directory use and untaint
    $item_photo = join "", $adID, $extensioncheck;
    $item_photo =~ tr/ /_/;  
    $item_photo =~ s/[^$safe_filename_characters]//g;  
    if ( $item_photo =~ /^([$safe_filename_characters]+)$/ ) { $item_photo = $1; }
    else {  die "Filename contains invalid characters"; }  
    }

$adID is generated by the script itself using a localtime(time) function, so it should not be tainted. $item_photo is reassigned using $adID and $extensioncheck BEFORE the taint check, so the new $item_photo is now untainted. I know this because $item_photo itself has no problem with unlink itself latter in the script. $item_photo is only used long enough to create three other image files using ImageMagick before it's tossed using the unlink function. The three filenames created from the ImageMagick processing of $item_photo are created simply like so.

$largepicfilename  = $adID . "_large.jpg";
$adpagepicfilename = $adID . "_adpage.jpg";
$thumbnailfilename = $adID . "_thumbnail.jpg";

The paths are prepended to the new filenames to create the URLs, and are defined at the top of the script, so they can't be tainted as well. The URLs for these files are generated like so.

my $adpageURL = join "", $adpages_dir_URL, $adID, '.html';
my $largepicURL  = join "", $adphotos_dir_URL, $largepicfilename;
my $adpagepicURL = join "", $adphotos_dir_URL, $adpagepicfilename;
my $thumbnailURL = join "", $adphotos_dir_URL, $thumbnailfilename;

Then I write them to the record, knowing everything is untainted.

Now comes the screwy part. In a second script I read these files in to be deleted using the unlink function, and this is where I am getting my 'Insecue dependency' flag.

# Read in the current Ad Records Database
open (ADRECORDS, $adrecords_db) || die("Unable to Read Ad Records Database");
flock(ADRECORDS, LOCK_SH);
seek (ADRECORDS, 0, SEEK_SET);
my @adrecords_data = <ADRECORDS>;
close(ADRECORDS);

# Find the Ad in the Ad Records Database
ADRECORD1:foreach $AdRecord(@adrecords_data)
{
  chomp($AdRecord);
  my($adID_In, $adpageURL_In, $largepicURL_In, $adpagepicURL_In, $thumbnailURL_In)=split(/\|/,$AdRecord);

  if ($flagadAdID ne $adID_In) { $AdRecordArrayNum++; next ADRECORD1 }
  else
  {
    #Delete the Ad Page and Ad Page Images
    unlink ("$adpageURL_In");
    unlink ("$largepicURL_In");
    unlink ("$adpagepicURL_In");
    unlink ("$thumbnailURL_In");
    last ADRECORD1;
  }
}

I know I can just untaint them again, or even just blow them on through knowing that the data is safe, but that is not the point. What I want is to understand WHY this is happening in the first place, as I am not understanding how this previously untainted data is now being seen as tainted. Any help to enlighten where I am missing this connection would be truly appreciated, because I really want to understand this rather than just write the hack to fix it.

+4  A: 

Saving data to a file doesn't save any "tainted" bit with the data. It's just data, coming from an external source, so when Perl reads it it becomes automatically tainted. In your second script, you will have to explicitly untaint the data.

After all, some other malicious program could have changed the data in the file before the second script has a chance to read it.

Greg Hewgill
Have to go over my perlsec again, as I may have missed that point. Although in this case, the data is written to a folder with only owner permissions set, so another malicious program should not be able to change the data from an external program.
Epiphany
"so another malicious program should not be able to change the data". Well, there is no way to verify that. The taint checker needs to be super-paranoid, otherwise there is not much point in using it.
Thilo
@Greg and Thilo: Thanks guys... I guess I'm off to write another set of regexes so I can get on with it. Love the doo, Greg! :)
Epiphany
With very few exceptions, if it comes from outside the program, it is tainted.
Schwern