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.
kitten.php.gif
with the contentGIF89a<?php eval($_GET['shell']);
, accessing yourkitten.php.png
will run the PHP code, which will let me run my code. PHP runs every file that has.php
somewhere in the filename, which means that now I can execute arbitrary code on your server.