Re: File upload security
- From: Justin Koivisto <justin@xxxxxxxxx>
- Date: Wed, 27 Jul 2005 14:10:59 GMT
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 - justin@xxxxxxxxx
http://koivi.com
.
- References:
- File upload security
- From: Geoff Berrow
- File upload security
- Prev by Date: Re: Show countdown from 10 to 0 seconds
- Next by Date: Re: turn off register_globals on a shared server
- Previous by thread: File upload security
- Index(es):
Relevant Pages
|
