Maybe fixed: Taint bug

[Stephen Cox]

On Mon, Jul 16, 2012 at 3:17 PM, Sander Klein <roedie at roedie.nl> wrote:
> Hi All,
>
> I think I've found and fixed the taint bug which is in the mailscanner
> releases. The problem is in the PFDiskStore.pm file.
>
> The routine giving the problems is:
>
> sub CopyEntireMessage {
>    my $this = shift;
>    my($message, $targetdir, $targetfile, $uid, $gid, $changeowner) = @_;
>
>    #print STDERR "Copying to $targetdir $targetfile\n";
>    if (MailScanner::Config::Value('storeentireasdfqf')) {
>      #print STDERR "Copying to dir $targetdir\n";
>      return ($this->CopyToDir($targetdir, $targetfile, $uid, $gid,
>                               $changeowner));
>    } else {
>      #print STDERR "Copying to file $targetdir/$targetfile\n";
>      my $target = new IO::File "$targetdir/$targetfile", "w";
>      MailScanner::Log::WarnLog("writing to $targetdir/$targetfile: $!")
>        if not defined $target;
>      $this->WriteEntireMessage($message, $target);
>      return $targetdir . '/' . $targetfile;
>    }
> }
>
> Here $targetfile is still tainted and will result in failure. Changing
> te routine to:
>
> sub CopyEntireMessage {
>    my $this = shift;
>    my($message, $targetdir, $targetfile, $uid, $gid, $changeowner) = @_;
>
>    $targetfile =~/([\w\d]{10}.[\w\d]{5})/;
>    $targetfile = $1;
>
>    #print STDERR "Copying to $targetdir $targetfile\n";
>    if (MailScanner::Config::Value('storeentireasdfqf')) {
>      #print STDERR "Copying to dir $targetdir\n";
>      return ($this->CopyToDir($targetdir, $targetfile, $uid, $gid,
>                               $changeowner));
>    } else {
>      #print STDERR "Copying to file $targetdir/$targetfile\n";
>      my $target = new IO::File "$targetdir/$targetfile", "w";
>      MailScanner::Log::WarnLog("writing to $targetdir/$targetfile: $!")
>        if not defined $target;
>      $this->WriteEntireMessage($message, $target);
>      return $targetdir . '/' . $targetfile;
>    }
> }
>
> fixes the problem. To make this change perfect it probably would be
> better to check if $targetfile isn't empty after setting $1. This is
> just my own POC to see if the change works.
>

You are welcome to open a pull request on github @
https://github.com/MailScanner/MailScanner

> Greets,
>
> Sander
>
> --
> MailScanner mailing list
> mailscanner at lists.mailscanner.info
> http://lists.mailscanner.info/mailman/listinfo/mailscanner
>
> Before posting, read http://wiki.mailscanner.info/posting
>
> Support MailScanner development - buy the book off the website!



-- 
Stephen Cox