2

Question is simple. How can I make 100% safe photo upload script with php? Is there any tutorials which shows all possible safeness's gaps?

Do not offer me to look at this question, because there they talk only about size. But I want to be sure, that nobody can upload shell and other stuff. Because it's a big website which need 100% safe photo upload script.

Edit: or maybe I should allow members to upload pics to receptacles like imageshack and copy their link to my website? I guess it is 100% safe, right?

Community
  • 1
  • 1
good_evening
  • 21,085
  • 65
  • 193
  • 298
  • 4
    "100% safe"? Does not exist. Name one system that was thought to be secure that was not cracked. – Matchu Jun 24 '10 at 21:50
  • 1
    You mean like Facebook? – Jaymz Jun 24 '10 at 21:51
  • @hey - I think he's pointing out that even Facebook's system is not 100% safe. – Matchu Jun 24 '10 at 21:53
  • I see. Now please view the edited version. Am I safe if I allow people to upload their photos somewhere else and paste their links to input and I will save it in mysql? – good_evening Jun 24 '10 at 21:56
  • You're merely assuming that "somewhere else", like ImageShack, is "100% safe" at this point. – Cal Jacobson Jun 24 '10 at 21:58
  • 3
    Secure file upload is **hard**. See http://stackoverflow.com/questions/602539/stop-people-uploading-malicious-php-files-via-forms for some issues. Yes, if you can get away with imageshack et al, do that. – bobince Jun 24 '10 at 22:15

3 Answers3

4

It's really rather simple. Run all uploaded images through an image filter that is known to be safe. If it kicks back with a "Not an image error", you have shenanigans. (A simple example would be an identity transform, or a JPEG quality normalization technique.) An important point, tho', is to actually use the output from the filter, not the original file.

Williham Totland
  • 28,471
  • 6
  • 52
  • 68
  • 1
    Note that WMF's were both images and executables. Which led to malware guys embedding stuff inside valid "images"..but that was a while back. – NotMe Jun 24 '10 at 21:54
  • It's not that simple, images can contain e.g. PHP code in comments and still be valid images. – Christian Davén Mar 02 '11 at 09:15
  • 1
    @Christian Davén: Using a more aggressive filter; particularly one that resamples and reencodes the image, stripping metadata et. al. should take care of most stuff; as should tools like pngcrush. – Williham Totland Mar 02 '11 at 09:44
4

Things you need to consider and watch out for:

File extensions: Webservers use the file extension to determine the MIME-Type sent to the client. You should use a whitelisting of extensions that you allow. In your case this would be image extensions.

LFI: As already mentioned by Matchu, local file inclusion can be an issue. If your application has dynamic includes that allow you to include an arbitrary file, eg. include($_GET['myfile']); it can lead to arbitrary PHP execution. So you need to secure such includes by using basename(). This can even happen with images, since they can contain embedded comments.

MIME-Type-Detection: This is actually not very well known, and there's not much information about it. IE<8 has a feature called MIME-Type-Detection which will, if the Content-Type does not match with the content, try to guess the type by looking at the first 256 bytes. This means, if you have a PNG file called image.gif with a comment at the top that reads <script>alert('hello');</script>, then IE<8 will think it is HTML and execute the JavaScript, leading to an XSS attack.

To prevent this issue for images you can use the approach mentioned by Williham Totland. I would suggest using getimagesize() to detect if the image is valid and to ensure that the extension matches the type.

$image_filename = 'uploaded_image.gif';
$image_extension = substr(strrchr($image_filename, '.'), 1);

$mime_mapping = array('png' => 'image/png', 'gif' => 'image/gif', 'jpg' => 'image/jpeg', 'jpeg' => 'image/jpeg');
$info = getimagesize($image_filename);

if (!$info) {
    exit('not an image');
}

if ($info['mime'] != $mime_mapping[$image_extension]) {
    exit('wrong extension given');
}

// all checks passed
// image can be saved now

There is an article written about this issue here.

igorw
  • 27,759
  • 5
  • 78
  • 90
  • Note that it's possible to make an file that contains ` – bobince Jun 25 '10 at 11:32
  • Also there are other types you might try to smuggle into an image, in particular Java and Flash code. These allow scripting but do not abide by the exact ‘same origin policy’ that JavaScript does, so by planting a JAR or SWF on a target server it is partially possible to cross-site-script into it from another page. See the ‘GIFAR’ attack for an example of this. For these reasons it is difficult to guarantee that file upload will be ‘safe’ even with precautions. A good response is to put all user-uploaded content on a separate hostname that does not share a security context with the main site. – bobince Jun 25 '10 at 11:36
  • IE will only use MIME-Type-Detection if image content and headers don't match, which my implementation checks. The issue is that there is no such signature for PNG images in older versions. And you are right, GIFAR and flash are quite an issue that require a custom serving script. [Possible solution for GIFAR][1]. Flash attacks require the serving script to be in a separate directory and if possible without public filenames. Or your solution, of course. [1]: http://github.com/phpbb/phpbb3/commit/bf59a749c3346ed7341d03946b8ecd0701af9eb8 – igorw Jun 25 '10 at 12:11
  • I would give unique name to an image, so I guess I will prevent attacks such as XSS. – good_evening Jun 25 '10 at 12:19
2

As you mentioned, there's another question that addresses the issue. The author notes that he should be safe as long as he only accepts genuine image files and keeps down the filesize, which is about the only thing you can do. Check extension and MIME type and reject anything that does not match the formats you want. There's still RFI (well, more like LFI here), but as long as you aren't haphazardly including files without performing sanity checks, you should be good to go.

Community
  • 1
  • 1
Matchu
  • 83,922
  • 18
  • 153
  • 160