5

Let's say you could upload any file you wished to a server, but the file extension MUST be ".jpg". Would you be able to upload anything that could harm the server?

The point of my question is that file type verification is slow, and I would rather only have to check the file extension if that is secure enough. I am having trouble imagining a scenario where malware disguised as an image could be used.

I have seen recommendations for using getimagesize() to verify an image, but this function is pretty slow, and I cannot figure out if it is necessary, or even effective, for preventing malware uploads...

Any information on this is greatly appreciated.

hakre
  • 193,403
  • 52
  • 435
  • 836
dqhendricks
  • 19,030
  • 11
  • 50
  • 83
  • Won't it work by checking the MIME type ? You don't really want to have PHP scripts on yours server, even if with a jpg extension they aren't that dangerous (and I'm not even sure of that) – Julien Jun 17 '11 at 21:17
  • 4
    I doubt the slowness of the file type check is comparable to the http upload process itself. – mario Jun 17 '11 at 21:18
  • the speed of the function shouldn't matter since you'll run it directly after upload which no user expects to be instant anyways. I'd say just use http://www.php.net/manual/en/book.fileinfo.php and be done with it and you'll never have to worry. – castis Jun 17 '11 at 21:19
  • @mario do you think that fileinfo, or getimagesize would even be effective in preventing malware? – dqhendricks Jun 17 '11 at 21:25
  • I think *that* depends on the malware. – hakre Jun 17 '11 at 21:40
  • 1
    @dqhendricks: Actually you can hide code in a perfectly valid JPEG or PNG file. Basic magic byte verification however helps as workaround for IEs type sniffing vulnerabilities. – mario Jun 17 '11 at 21:48
  • See as well: [PHP Upload file enhance security](http://stackoverflow.com/q/2751384/367456) – hakre Dec 26 '11 at 18:06

4 Answers4

4

For testing the security of images, I really only use one method: just re-create the image with GD or whatever image processing library you're using, and use the new image.

This should make the file perfectly secure if you have updated versions of your library.

It may make your stuff a bit slower, but the safer the better.

Just checking if there is a '.jpg' at the end of a file name will do nothing. The file can still be any type of file.

Hope this helped!

Jeff Gortmaker
  • 4,607
  • 3
  • 22
  • 29
  • 2
    And conversely, if you have outdated versions of the library, you're screwed if there are exploits. – Chris Thornton Jun 17 '11 at 21:19
  • True! Personally I'd rather rely on the person who made GD's coding then on mine, so I'd say even an outdated version of an image processing library would be preferable! – Jeff Gortmaker Jun 17 '11 at 21:21
  • Any library will fail if you send in a faked image that has an enourmous size within it's headers for the image size but is small in size. Send a bunch of upload requests and it will eat resources on the server. – hakre Jun 17 '11 at 21:39
  • @hakre, that "rate-limiting" should be handled at the server-level - not the application/PHP level. – Xeoncross Jan 05 '12 at 16:45
  • I could imagine that GDlib will request the memory from the system - and depending how it's coded - this can go over the configured PHP memory limit. Just noting, I don't know if this is the case for the gd library in PHP. Limiting the PHP process memory limit is always wise, next to PHP's own memory limit setting. – hakre Jan 05 '12 at 16:52
4

It seems like only checking the extension is unsafe. Harmful to the server? Hard to say, without knowing the server OS, permissions on the uploaded files, etc.. You definitely don't want to allow these to be executed. It's certainly easily abused by allowing users to upload non-images, then they just rename to what it really is when their friends download it. This is how you find yourself unwittingly hosted pirated movies, warez, etc..

Chris Thornton
  • 15,620
  • 5
  • 37
  • 62
  • I think that's the foremost important point even. It's an old trick. Another one is to fake the file headers or append it to the original file. – hakre Jun 17 '11 at 21:31
3

If you think getimagesize() is a bit too slow (because all uploads are done in super highspeed as we know ;) ) you can try the fileinfo library as well. It inspects at least some bytes within the file. It's pretty fast, I use it every day for hundreds of files in an app that should run speedy and it does.

However, what you don't verify you don't know. So probably first checking extension, ensure a safe filename and a safe store and that they are properly send out to the client.

Before letting any image library touch it (and this should include those on the computers of your site's users), for security reasons the file should be scanned by a virus scanner. That's much more slow compared to getimagesize(), others suggest to take a look into the file for any occurance of <?php as well to prevent uploading as payload. Naturally this includes checking for phar files if inclusion is not prevented via the PHP installations security settings (e.g. by suhosin)

Next to on-demand virus scanning, stored files should be checked from time to time again and again because of formerly unknown exploits.

So part of this is always a background job. But even the on demand real-time checks often do not take that much time unless your application does uploads all the time. You might want to introduce some upload-queue, so the upload is already done but the file get's available to the uploader after the necessary tasks have been run.

hakre
  • 193,403
  • 52
  • 435
  • 836
0

Another side-effect of not checking whether it's an image or not is when, and if, you need to display it somewhere on the website (in user profile, on front page, etc) then if it's not really an image, it will appear broken, and thus, your site will appear broken.

As for malware being injected, that depends on your server configuration. If it hasn't been updated to secure holes and exploits then yes, malware can infiltrate through (even though that's rare, but possible).

Sawant
  • 4,321
  • 1
  • 27
  • 30