File upload security

[Geoff Berrow]

I've written a script which takes a couple of user image files and uses
them to create a watermarked image.

http://www.ckdog.co.uk/watermark.php

I'm checking the images like so:

if(!empty($_FIL ES['pattern']['tmp_name']) ){
if(is_uploaded_ file($_FILES['pattern']['tmp_name'])){
if(
!$patternsize=g etimagesize($_F ILES['pattern']['tmp_name'])){
header('Locatio n: watermark.php?n osize=2');
exit;
}
$patt=$_FILES['pattern']['tmp_name'];
}
}

It has been suggested to me that this is still insecure as people could
use 'character substitution hacks' to upload files to the tmp directory.

Comments?
--
Geoff Berrow (put thecat out to email)
It's only Usenet, no one dies.
My opinions, not the committee's, mine.
Simple RFDs http://www.ckdog.co.uk/rfdmaker/

[Justin Koivisto]

Re: File upload security

Geoff Berrow wrote:
[color=blue]
> I've written a script which takes a couple of user image files and uses
> them to create a watermarked image.
> http://www.ckdog.co.uk/watermark.php
>
> I'm checking the images like so:
>
> if(!empty($_FIL ES['pattern']['tmp_name']) ){
> if(is_uploaded_ file($_FILES['pattern']['tmp_name'])){
> if(
> !$patternsize=g etimagesize($_F ILES['pattern']['tmp_name'])){
> header('Locatio n: watermark.php?n osize=2');
> exit;
> }
> $patt=$_FILES['pattern']['tmp_name'];
> }
> }
>
> It has been suggested to me that this is still insecure as people could
> use 'character substitution hacks' to upload files to the tmp directory.
>
> Comments?[/color]

I usually start out like this (from the top of my head, not tested)...

$IMG_PROCESSED= FALSE;
if( isset($_FILES['pattern'])
&& isset($_FILES['pattern']['error'])
&& $_FILES['pattern']['error']==0
){
// this tells me that the file was uploaded via my script
if(file_exists( $_FILES['pattern']['tmp_name'])){
// the file exists
if($size=getima gesize($_FILES['pattern']['tmp_name'])){
// See notes below
$IMG_PROCESSED= TRUE;
}
}
}

if(!$IMG_PROCES SED){
// whatever error stuff needs to be done goes here
}

The next steps I usually do involve checking that the image type is
something that I am expecting and that the server's PHP/GD install can
handle. Once I have decided that everything is OK, I set the
$IMG_PROCESSED variable to boolean true.

To be honest, I haven't really dealt with character substitution hacks
because it has never come up for me. However, I don't see how character
substitution would get by checking with file_exists, getimagesize, and
then parsing the output from getimagesize... If that is still insecure,
I'd like to hear about that. (Perhaps Chris Shiflett would be the one to
really answer that question...)

--
Justin Koivisto, ZCE - justin@koivi.co m

http://koivi.com