34

A colleague of mine has a personal website in which he allows users to upload anything within a certain size,

but before the actual upload he checks to see the file extension:

if ( $type == 'image/gif'){
  $ext = '.gif';
} elseif ( $type == 'image/jpeg'){
  $ext = '.jpg';
} elseif ( $type == 'image/png' ){
  $ext= '.png';
}  else {
  $ext = '.png';  
}

He says to me, that by making all files images no harm can be done to the server.

for example :

  evilscript.evil

would become :

evilscript.png

And be thereby rendered useless. Is there any truth to his claims?

puzzlepiece87
  • 313
  • 1
  • 7
Mister Verleg
  • 501
  • 5
  • 7
  • 8
    This only defends against the most trivial of attacks. If we're only talking about image files, then strip the EXIF data and convert them to a different format. – symcbean Aug 09 '16 at 11:00
  • 3
    I don't think that is what he is referring to. If I got this right he was talking about having a script file being renamed to an image (--> non-script) extension. If the content is still a script, you can't strip exif data, just try to prevent one single way of executing it (out of many). – Draugr Aug 09 '16 at 11:53
  • 7
    Even if you pass only image files to image processors you're still not safe from everything. For example: https://imagetragick.com/ – OrangeDog Aug 09 '16 at 16:04
  • What is the server's OS? – enkryptor Aug 09 '16 at 18:00
  • 18
    If I send a file called `kitten.php.gif` with the content `GIF89a – Ismael Miguel Aug 10 '16 at 08:36
  • "but before the actual upload" - I think you mean "but _before_ it's moved from its temporary location", the file has already been uploaded at this point? Aside: Rather than simply setting a file extension of `.png` for all essentially _unknown_ files - the request should probably be aborted and the file not saved at all. (?) – MrWhite Aug 10 '16 at 08:46
  • 7
    @IsmaelMiguel "PHP runs every..." It's not PHP which decides which files to execute but the HTTP server e.g. Apache, NGINX, etc.. It's usually a good idea to configure the server to only execute white-listed file types, or have custom routing decide what to execute. – noahnu Aug 10 '16 at 16:08
  • 2
    With a sane framework and configuration you could upload any file with any extension and still be safe (at least on the server - the file could do damage to the clients but that's a different matter). If you let the web server execute every file that has a php extension then you're in trouble. Instead, use a router (have the web server redirect every request to the router's php file) and separate your code from the upload folder, so there's no way a malicious file could be called and executed. – André Borie Aug 10 '16 at 16:20
  • 2
    @noahnu You're correct. I should have worded it differently. Something along the lines of "Webservers are configured (by default) to pass to PHP any fine that *somewhere* in the name has `.php`." would have been more accurate. – Ismael Miguel Aug 10 '16 at 19:00
  • http://demoseen.com/blog/2011-08-31_Superpacking_JS_Demos.html AND http://lcamtuf.coredump.cx/squirrel/ – Brad Werth Aug 11 '16 at 02:41
  • @IsmaelMiguel Can you name a common setup where that is the default? As far as I know, it really has to end in `.php` (or another known extension, sometimes `.php5` is also used, or perhaps some others) and having a known extension *anywhere* in the filename is definitely not sufficient. You scenario does not strike me as a common setup, and I never heard of anyone testing for it (at CTFs or in security firms mainly). The only vaguely realistic scenario I can think of would be `.php.gz` or so, where the final part is known and decoded, and then passed to PHP, but I've never seen that either. – Luc Jul 15 '18 at 23:00
  • @Luc I've seen it. Maybe what I've seen wasnt, afterall, that very common. But I remember seeing that some years ago. – Ismael Miguel Jul 16 '18 at 11:47

5 Answers5

50

There are basically two main ways an uploaded file can be harmful: by being executed (as a script or binary) or by being run/used in an application and abusing an exploit in it (e.g. an uploaded MP3 which is then opened by a specific player, abusing a known weakness in it).

In other words, it all depends what happens with the file after uploading. If someone is able to upload a Perl script, but the server never executes the uploaded files or even does not have Perl installed, nothing can ever happen. In general: if you make sure that the uploaded file is never run or interpreted you will be safe.

Renaming the files only helps with one thing: on some operating systems, some file extensions may be linked with a specific application. If you rename the file you might prevent that the file will be opened with a linked application. But depending on the system and setup the uploaded files might still get opened with a vulnerable application. To stay in the above example: if any uploaded file gets opened with an MP3 player, even if you rename it to song.png, it would still be able to exploit a weakness in the player (except if the player has its own layer of checking and e.g. does only accept .mp3 files).

The renamed files are not images suddenly, just because of the renaming. On Unix and similar systems, there is even the file command to analyze the type/MIME type of a file.

Bottom line: in my eyes there is only one thing you can do. Be very specific in your setup about what can and will be done with the uploaded files. Any libraries, extensions or applications accepting these files should always be updated to the latest version.

Toby Speight
  • 1,214
  • 9
  • 17
Draugr
  • 670
  • 9
  • 14
  • 9
    Some webservers are typically configured to handle files based on their extension - for example, running all .php files through PHP. On such a configuration, it makes perfect sense to validate extensions and make .php not an allowed extension. – user253751 Aug 09 '16 at 10:30
  • 8
    You're right, that's what I was referring to with "be very specific in your setup about what can and will be done with the files". Validating extensions is one part of this, but by far not the only thing, that's why I didn't want to mention this specifically. – Draugr Aug 09 '16 at 10:33
  • 5
    The information in this answer is very misleading. It's trivial to upload PHP source bypassing the *intent* of this code which will be executed by Apache with content negotiation enabled (and possibly on other platforms too). see http://symcbean.blogspot.co.uk/2016/06/local-file-inclusion-why-everything-you.html – symcbean Aug 09 '16 at 10:53
  • 6
    I don't think what I wrote is misleading. A LFI via Apache could only be abused if you allow a file upload and enable Apache to interpret files in the uploaded directory. If you just have the uploaded file stored and the httpd is not configured to include this part it you can't execute the php source you mentioned. That's why I phrased it on an abstract level: be specific and sure about each processing step and possibility of your uploaded files. This includes a running httpd. – Draugr Aug 09 '16 at 11:49
  • 1
    @Draugr: You might however want to make your answer more specific to webservers serving these files (and their many underhanded vulnerabilities), instead of using that contrived music player example. – Bergi Aug 09 '16 at 21:53
12

No. Renaming a file doesn't increase security.

He says to me, that by making all files images no harm can be done to the server.

for example evilscript.evil would become evilscript.png

When you rename evilscript.evil to evilscript.png you don't turn it into an image. You just change its name. Generally, a file name isn't relevant. It is just a name given to a block of data, nothing more.

If you can execute an uploaded script, you probably can do it regardless of its name. If you cannot, uploading a malicious script doesn't harm the system, since the script won't be executed anyway.

However, it can prevent a file from being accidentally run. The only protection renaming could provide is the protection from being accidentally launched by Windows explorer (or a shell that similarly uses file extensions). So renaming virus.exe into virus.exe~ actually helps, when you accidentally tap Enter on it.

Unix shells use file formats instead of extensions. As an example, you can save a script as evilscript.png and run it with a Linux shell, providing the file has the "execute" permission. In terms of security, generally it is better to control file permissions instead of file names.

enkryptor
  • 313
  • 1
  • 10
  • 1
    So in conclusion, does preventing a file from being accidentally run increase security compared to being able to accidentally run it? – nitro2k01 Aug 09 '16 at 17:46
  • 6
    @nitro2k01 it does, primarily for a desktop system; but it's a protection from a human error, nothing more – enkryptor Aug 09 '16 at 17:50
  • 1
    1) We're talking about *uploads* here, not *downloads*. 2) If there is at least one attack vector that relies on the extension, then changing the extension does *increase* security (even if it doesn't increase it "enough"). Since Apache does use extensions to determine script types, there is a known, common, case where renaming a file does indeed increase security. – IMSoP Aug 10 '16 at 18:08
  • 2
    @IMSoP what makes you think I was talking about downloads? – enkryptor Aug 10 '16 at 18:09
  • @enkryptor Because you talk about Windows Explorer and Unix shells, both of which are only relevant once the file has been downloaded to some client. – IMSoP Aug 10 '16 at 18:13
  • 2
    A shell script can be executed server-side, actually.. so do a server administrator can accidentally run an uploaded malicious script. Can't argue with the Apache argument though. – enkryptor Aug 10 '16 at 18:18
  • Downvoted because I think it does increase security (in fact, it 100% mitigates/fixes the vulnerability that this question is about) and because you don't have to scream your opinion in a large+bold font. See my answer. – Luc Jul 15 '18 at 23:38
2

Checking the only extension of an uploaded file is not enough. Eg: check the answer on this question.

A proposal: There is a more reliable way to find out the type of a file. If your friend is running the server on a Unix machine you could use the 'file' command which inspects the contents of the file to determine the format.

George Gkitsas
  • 386
  • 1
  • 3
  • 8
    No. Validating the mimetype is useful but provides no protection against malware *embedded* in the content. – symcbean Aug 09 '16 at 10:55
  • 2
    The `file` command has had a number of security vulnerabilities (mostly classified as DoSes and not necessarily code execution). I don't love the idea of running it on untrusted data. Parsing dozens of file types? ~shudder~ – Matt Nordhoff Aug 09 '16 at 17:26
  • 1
    How do you propose to distinguish an `application/zip` from an `application/vnd.openxmlformats-officedocument.spreadsheetml.sheet` from an `application/java-archive` from all the other files that are "zip archive with structured content"? – Mark Aug 09 '16 at 20:57
2

There are two isssues, protecting against direct attacks on the server and protecting against attacks on other clients (which in turn can lead to attacks on the server)

First the server, when a client requests a file the server has to decide whether to serve it or execute it somehow. This descision may depend on various factors including.

  1. The file extension
  2. The directory the file is in
  3. Whether the executable bit is set.

I'm not aware of any web servers doing content-inspection on the server side but I can't rule out that one may exist.

Ideally on the server you would not put user-supplied files in a directory where script execution is allowed in the first place, but restricting the file extension is likely a sensible "defense in depth" policy in case a file inadvertantly ends up in the wrong place.

If the server does decide to serve the file it must then decide what mime type to send. Afaict servers usually choose the mime type based on the file extension. The client must then decide what to do with the file. It should base this on the mime type sent by the server but in practice it may instead base it on the file extension or on inspecting the content of the file,

If the browser chooses to treat the file as a type that can involve client side scripting then the domain the file is served from becomes important. If the file is served from your main domain then the script can likely read the cookies you set and use them to impersonate the user.

I belive that a file-extension restriction should be part of the security strategry but it should not be all of it. Storage in directories that forbid script execution, enforcement of contents matching extension and use of a seperate domain name should all be considered as part of the overall defense strategy.

Peter Green
  • 4,918
  • 1
  • 21
  • 26
  • 1
    This much more directly addresses the situation described in the question than any of the existing answers. And "defence in depth" is an important point: It would be just as dangerous to say "you can safely ignore file extensions" as it would be to say "if you fix the file extension, you can safely ignore everything else". – IMSoP Aug 10 '16 at 18:11
1

This is safe.

One can always smuggle executable code inside of an image:

So you should already assume that your file's contents are not safe when they are user-supplied. Even if you re-encode the image, and even if you convert formats, it's still an image so don't bloody execute it. If your web server (such as Apache or Nginx) runs it through the PHP interpreter, then someone will find a way to trigger PHP to do something nasty. By forcing the extension to be one of a few whitelisted extensions, you can tell your web server that it is an image (not executable) and that it should not go through the interpreter.

There are, of course, some related risks to be aware of:

  • Your user might put null bytes or other funny things in the filename, which might allow them to choose their extension after all.
  • Your user might try to use path traversal if you let them choose the filename, which might allow them to do all sorts of things.
  • You might make a mistake in the web server's configuration and everything gets run through the interpreter regardless of the extension (I've seen this before: it's a silent bug, because PHP literally outputs anything it doesn't recognize, so images will look the same before and after PHP interpretation).
  • The file might contain an exploit for a specific decoder, for example if you know that the admin might view your image and that they use a browser which has a vulnerable PNG viewer, you could upload an PNG image that exploits that vulnerability.

The first two are easily solved: never let the user choose their filename, or at least whitelist a set of characters that are definitely safe, such as ^[a-zA-Z0-9_-]*$ (alphanumeric, underscore, and hyphen).

The third one can be checked for by putting a file called "test.png" somewhere and putting <?php echo "test1"; in it. When requesting the file via curl, you should see the full code. If you only see "test1", then it's vulnerable.

Finally, the last one is hard to check for and somewhat out of scope. You could use a virus scanner on the server side, but virus scanners only catch a few common cases and will usually not be able to detect all variants of a vulnerability, if they know about the vulnerability in the first place. The user should, ideally, update their system and not run vulnerable software. But if you're in a high-security environment, then you should apply defense in depth and do things like re-encode the image, perhaps only let users view images in a virtual machine, etc. But that's out of scope for this question.

Summary: There are some related attacks that you have not shown to be mitigated, but when only considering the code snippet as posted: that part is safe.

Luc
  • 31,973
  • 8
  • 71
  • 135