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($_FILES['pattern']['tmp_name']) ){
if(is_uploaded_file($_FILES['pattern']['tmp_name'])){
if(
!$patternsize=getimagesize($_FILES['pattern']['tmp_name'])){
header('Location: watermark.php?nosize=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]

Geoff Berrow wrote:

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($_FILES['pattern']['tmp_name']) ){
if(is_uploaded_file($_FILES['pattern']['tmp_name'])){
if(
!$patternsize=getimagesize($_FILES['pattern']['tmp_name'])){
header('Location: watermark.php?nosize=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?

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=getimagesize($_FILES['pattern']['tmp_name'])){
// See notes below
$IMG_PROCESSED=TRUE;
}
}
}

if(!$IMG_PROCESSED){
// 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 - ju****@koivi.com
http://koivi.com