45

My client wants a photography site where users can upload their photos in response to photography competitions. Though technically this isn't a problem, I want to know the risks associated with allowing any user to upload any image onto my server. I've got the feeling the risks are high...

I was thinking of using something like this http://www.w3schools.com/php/php_file_upload.asp

If I do let anonymous users upload files, how can I secure the directory into which the images (and potentially malicious files) will be uploaded?

penalosa
  • 143
  • 6
Starkers
  • 553
  • 1
  • 5
  • 6
  • 2
    Josh did a very good job of answering this question. http://security.stackexchange.com/a/32853/31943 I think one should also read the guide mentioned here: http://stackoverflow.com/a/7065880/181562 which is: http://blog.insicdesigns.com/2009/01/secure-file-upload-in-php-web-applications/ – CrandellWS Oct 14 '13 at 00:58

2 Answers2

79

The biggest concern is obviously that malicious users will upload things that are not images to your server. Specifically they might upload executable files or scripts which they will attempt to trick your server into executing.

One way to protect against this is to make sure that the files are not executable after you move_uploaded_file in PHP. This is as simple as using chmod() to set 644 permissions.

Note that a user can still upload PHP scripts or other scripts and trick Apache into executing them depending on your configuration.

To avoid this, call getimagesize() on the files after they are uploaded and determine what file type they are. Rename the files to a unique filename and use your own extension. That way, if a user uploads evil.jpg.php, your script will save that as 12345.jpg and it won't be executable. Better yet, your script will not even touch it as it will be an invalid JPEG.

I personally always rename uploaded images to either the current timestamp from time() or a UUID. This also helps prevent against very evil filenames (like someone trying to upload a file they've named ../../../../../../../../etc/passwd)

As further protection you can use what's sometimes known as an "Image Firewall". Basically this involves saving uploaded images to a directory which is outside the Document Root and displaying them via a PHP script which calls readfile() to display them. This might be a lot of work but is the safest option.

A secondary concern is users uploading too many files or files which are too large, consuming all the disk space available or filling the hosting user's quota. This can be managed within your software, including limiting the size of individual files, the amount of data one user can upload, the total size of all uploads, etc. Make sure the website admin user has a way to manage and delete these files.

Do not rely on any of the data in $_FILES. Many sites tell you to check the mime type of the file, either from $_FILES[0]['type'] or by checking the filename's extension. Do not do this. Everything under $_FILES with the exception of tmp_name can be manipulated by a malicious user. If you know you want images only call getimagesize as it actually reads image data and will know if the file is really an image.

Do not rely on checking the HTTP Referrer for any security. Many sites advise you to check to make sure that the referrer is your own site to ensure that the file is legitimate. This can easily be faked.

For further information, here are some good articles:

Josh
  • 1,096
  • 9
  • 13
  • Nice answer, but now I want to look at the source for `getimagesize()` to see if I can subvert it :P – lynks Mar 19 '13 at 15:03
  • 1
    @lynks you probably can to some degree. But my point was it detects and returns the file and mime type of the image, and from that your code can save the file as `.jpg`, `.png`, `.gif` or reject it. On a properly configured Apache server, none of those file types will be executed as PHP / CGI scripts. Compare that to using the mime type from `$_FILE` which might be a lie, or trusting the filename sent from the client. – Josh Mar 19 '13 at 15:47
  • 1
    @lynks, [have at it](https://github.com/php/php-src/blob/master/ext/standard/image.c#L1292). – Adi Mar 19 '13 at 16:05
  • I once attacked a site by including `` in the text metadata for a PNG file. So the filetype check isn't a 100% reliable protection. However when combined with the other things like file extension and non-execute flags it should be OK. – Mike Weller Mar 20 '13 at 13:01
  • 1
    @MikeWeller When you did so, do you remember what the filename was? I'd be willing to bet it was `something.php.jpg` or even worse, `something.php`. A properly configured Apache server will not execute PHP code inside a file which does not have `.php` in the name. – Josh Mar 20 '13 at 16:57
  • You can use http://php.net/manual/en/function.exif-read-data.php instead of `getimagesize()`, if `getimagesize()` is a concern. – Dave Jarvis May 30 '13 at 22:41
  • If you use PHP as a module, you don’t need execution permissions, read permissions suffice. – Gumbo Mar 02 '14 at 10:46
  • Do you have any comments about @Rodney's answer stating that `getimagesize()` might be insecure? And also what do you think about code provided by CertaiN in PHP documentation? – Templar Jul 04 '14 at 11:15
  • Actually reading the 1x1JPG hack article it also mentions that it might be not save but as long as you make sure that file name ends with appropriate extension it will be fine? – Templar Jul 04 '14 at 14:32
  • Say, I'm able to upload a php file, bypassing the restriction. If the server always responds with **Content-Type: image/png**, is it still possible to execute scripts? – 1lastBr3ath Aug 05 '15 at 10:35
  • @1lastBr3ath it depends on how you have it configured. If the webserver executes the file (I.E. If the file is treated as executable code because the extension is something like .php) then you are still not safe. – Josh Aug 11 '15 at 16:20
  • 7
    I know I'm a few years late, but just a word on getimagesize(). You shouldn't use it to validate whether something is an image. From the PHP docs "Note: This function expects filename to be a valid image file. If a non-image file is supplied, it may be incorrectly detected as an image and the function will return successfully. Do not use getimagesize() to check that a given file is a valid image. Use a purpose-built solution such as the Fileinfo extension instead." http://php.net/manual/en/function.getimagesize.php – David G Oct 25 '15 at 15:43
5

Consult https://www.owasp.org/index.php/PHP_Security_Cheat_Sheet#File_uploads for some important tips.

getimagesize() can be tricked into running PHP code.

http://www.php.net/manual/en/features.file-upload.php has a user comment by CertaiN that provides a better alternative with finfo(FILEINFO_MIME_TYPE)

Rodney
  • 141
  • 2
  • 3
  • 1
    Do you have a reference for the getimagesize() vulnerability? – Brian Low Apr 10 '16 at 05:37
  • 1
    The fileinfo based checkers are easily fooled. They only read the first few bytes of the file. PHP concatenated onto the end of an image will fool both methods (resizing the image should truncate the code). and its also possible to get around the file extension check - see http://symcbean.blogspot.co.uk/2016/06/local-file-inclusion-why-everything-you.html (also curious to find out more about getting getimagesize() to execute PHP). – symcbean Jul 04 '16 at 15:20
  • It's not really execute code on `getimagesize()`, but to fake a image header and include PHP commands on a file to bypass image type check and upload a shell. You can do that including a PHP shell on a legit image. See https://www.idontplaydarts.com/2012/06/encoding-web-shells-in-png-idat-chunks/ for some examples. – ThoriumBR Feb 09 '17 at 18:57