On Thu, 19 Oct 2000, Alexander Farber (EED) wrote:

> Matt Sergeant wrote:
> > Not multiple times, but let them upload and store in a temp file, which
> > you can store the filename as a hidden field. Use File::MkTemp to create
> > the filenames.
> 
> Thanks for the advice, but doesn't File::MkTemp have a race condition? 
> The subroutine File::MkTemp::mktemp does following (comments are mine):
> 
>    $keepgen = 1;
> 
>    while ($keepgen){
>  
>          # generate a random file name and put it into $template
> 
>          if ($dir){
>             $lookup = File::Spec->catfile($dir, $template);
>             $keepgen = 0 unless (-e $lookup); # isn't it a race?
>          }else{
>             $keepgen = 0;        # here it doesn't even check -e
>          }
>    
>          next if $keepgen == 0;       # also, why this check?
>    }
>    return($template);
> 
> This looks as a bad quality module to me or am I awfully wrong? 

Its only insecure if you don't use sysopen($fh, $newname, O_RDWR | O_EXCL
| O_CREAT) (and then get a new filename if that failed 'cos the file
existed).

File::Temp is a slightly more secure alternative, doing the above line for
you.

You should also take an MD5 hash of the contents of the file to ensure
they don't change in the lifetime of the request.

Sorry, but I don't mention these things because they are obvious to me
these days.

-- 
<Matt/>

    /||    ** Director and CTO **
   //||    **  AxKit.com Ltd   **  ** XML Application Serving **
  // ||    ** http://axkit.org **  ** XSLT, XPathScript, XSP  **
 // \\| // **     Personal Web Site: http://sergeant.org/     **
     \\//
     //\\
    //  \\

Reply via email to