views:

91

answers:

3

Hey everyone,

I'm implementing a user-based image uploading tool for my website. The system should allow any users to upload JPEG and PNG files only. I'm, of course, worried about security and so I'm wondering how the many smarter people than myself feel about the following checks for allowing uploads:

1) First white list the allowable file extensions in PHP to allow only PNG, png, jpg, JPG and JPEG. Retrieve the user's file's extension via a function such as:

return end(explode(".", $filename));

This should help disallow the user from uploading something malicious like .png.php. If this passes, move to step 2.

2) Run the php function getimageize() on the TMP file. Via something like:

getimagesize($_FILES['userfile']['tmp_name']);

If this does not return false, proceed.

3) Ensure a .htaccess file is placed within the uploads directory so that any files within this directory cannot parse PHP files:

php_admin_value engine Off

4) Rename the user's file to something pre-determined. I.E.

$filename = 'some_pre_determined_unique_value' . $the_file_extension;

This will also help prevent SQL injection as the filename will be the only user-determined variable in any queries used.

If I perform the above, how vulnerable for attack am I still? Before accepting a file I should hopefully have 1) only allowed jpgs and pngs, 2) Verified that PHP says it's a valid image, 3) disabled the directory the images are in from executing .php files and 4) renamed the users file to something unique.

Thanks,

+7  A: 

Regarding file names, random names are definitely a good idea and take away a lot of headaches.

If you want to make totally sure the content is clean, consider using GD or ImageMagick to copy the incoming image 1:1 into a new, empty one.

That will slightly diminish image quality because content gets compressed twice, but it will remove any EXIF information present in the original image. Users are often not even aware how much info gets put into the Metadata section of JPG files! Camera info, position, times, software used... It's good policy for sites that host images to remove that info for the user.

Also, copying the image will probably get rid of most exploits that use faulty image data to cause overflows in the viewer software, and inject malicious code. Such manipulated images will probably simply turn out unreadable for GD.

Pekka
Great answer. I'll just add that as an additional defence, serve the images from a throw-away domain other than the domain where you set cookies. That way, even the image somehow has executable client side code, browser's same origin policy will prevent much harm.
sri
@sri great idea, haven't thought of that!
Pekka
A: 

All the checks seem good, number 3 in particular. If performance is not an issue, or you are doing this in the background, you could try accessing the image using GD and seeing if it is indeed an image and not just a bunch of crap that someone is trying to fill your server with.

SimpleCoder
+3  A: 

Regarding your number 2), don't just check for FALSE. getimagesize will also return the mime type of the image. This is by far a more secure way to check proper image type than looking at the mime type the client supplies:

$info = getimagesize($_FILES['userfile']['tmp_name']);
if ($info === FALSE) {
    die("Couldn't read image");
}
if (($info[2] !== IMAGETYPE_PNG) && ($info[2] !== IMAGETYPE_JPEG)) {
    die("Not a JPEG or PNG");
}
Marc B
+1 missed one " though(die command line 3).
Thomas
woops. Good catch. answer fixed, and thanks.
Marc B