[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apache-modperl
Subject:    Re: Bug in CGI.pm when run under mod_perl
From:       Chip Turner <chip () zfx ! com>
Date:       1999-06-29 17:16:55
[Download RAW message or body]

Lincoln Stein wrote:
> 
> That blows a hole in my idea of using home directories as a solution
> to the world-writable upload directory problem.  If CGI.pm were really
> smart, it would defer looking for the upload directory if it detected
> it was running as root, and refuse to do uploads at all as root.  You
> want to avoid someone doign this:
> 
>       ln /etc/passwd /tmp/CGItemp99999
> 
> Because then if CGI.pm tries to upload to CGItemp99999 it will
> overwrite /etc/passwd as root!

Perhaps CGI.pm could unlink the file before it tries to write to it? 
Or, alternatively, try something like this:

sub new {
    my($package,$sequence) = @_;
    my $filename;
    my $fh = Symbol::gensym;
    for (my $i = 0; $i < $MAXTRIES; $i++) {
        my $tmp = sprintf("${TMPDIRECTORY}${SL}CGItemp%d",$sequence++);
	sysopen $fh, $tmp, O_CREAT | O_EXCL
	  or next;
    }
    # untaint the darn thing
    return unless $filename =~ m!^([a-zA-Z0-9_ '":/\\]+)$!;
    $filename = $1;
    return (bless \$filename, $fh);
}

The sysopen will fail if the file already exists (the O_CREAT and
O_EXCL), so you know that the $fh it returns is an already opened file
that hasn't clobbered anything.  It would require a few changes where
the TempFile objects are used, but I believe this will be secure (and
avoid race conditions as well).  If you don't use the already-opened $fh
then the possibility of a race condition exists (not to mention $fh not
having been closed, which would be nasty with modperl).

Thoughts?

Chip

-- 
Chip Turner                   chip@ZFx.com
                              Programmer, ZFx, Inc.  www.zfx.com
                              PGP key available at wwwkeys.us.pgp.net

[prev in list] [next in list] [prev in thread] [next in thread]