# JPEG Upload - Thread: https://externals.io/message/60470 - Original archive: https://news-web.php.net/php.internals/60470 - Started by Richard Lynch on 2012-05-05T16:32:55+00:00 - Messages: 10 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #60470 — Richard Lynch — 2012-05-05T16:32:55+00:00 URL: https://externals.io/message/60470#60470 >In > most systems you can upload *anything* with a .jpg extension and the > app will take it, so you can still include the file People don't use imagecreatefromjpeg() to be sure it isn't some ware or executable or PHP script disguised as a JPEG?! That's just crazy. And inexcusable in a framework. Somebody might be able to craft a "JPEG" that validates and still manages to somehow parse some PHP in the middle... Probably using JPEG comments so it's easier. But on should at least you'd have some kind of validation on user input! -- brain cancer update: http://richardlynch.blogspot.com/search/label/brain%20tumor Donate: https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=FS9NLTNEEKWBE --- ## #60473 — Ferenc Kovacs — 2012-05-05T17:29:59+00:00 URL: https://externals.io/message/60470#60473 · In reply to: #60470 > > > […] > > People don't use imagecreatefromjpeg() to be sure it isn't some ware > or executable or PHP script disguised as a JPEG?! > > That's just crazy. > > And inexcusable in a framework. > > Somebody might be able to craft a "JPEG" that validates and still > manages to somehow parse some PHP in the middle... Probably using JPEG > comments so it's easier. > > yeah, and injecting php code through the jpeg comments isn't new also, see http://ha.ckers.org/blog/20070604/passing-malicious-php-through-getimagesize/ but I bet I could find even older posts discussing the topic. so imo the correct remedy for this situation is to prevent your uploaded files to be executed at the first place, instead of trying to write an error-prone method to detect malicious content inside your uploaded media files. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #60475 — Richard Lynch — 2012-05-05T17:50:21+00:00 URL: https://externals.io/message/60470#60475 · In reply to: #60473 > > > > […] > yeah, and injecting php code through the jpeg comments isn't new also, > see > http://ha.ckers.org/blog/20070604/passing-malicious-php-through-getimagesize/ > but > I bet I could find even older posts discussing the topic. > so imo the correct remedy for this situation is to prevent your > uploaded > files to be executed at the first place, instead of trying to write an > error-prone method to detect malicious content inside your uploaded > media > files. getImageSize is not better than file Info... If the whole thing parses as an image with imagecreatefromjpeg() I should think that's a bit tougher to create a hack that works. Then one can strip off the exif info with the comments, I believe. And, yes, ideally one would keep images in a totally separate directory not even in the webtree... Which I do, but some folks can bear the cost of passing the image "through" PHP. -- brain cancer update: http://richardlynch.blogspot.com/search/label/brain%20tumor Donate: https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=FS9NLTNEEKWBE --- ## #60477 — Ferenc Kovacs — 2012-05-05T17:58:10+00:00 URL: https://externals.io/message/60470#60477 · In reply to: #60475 > > > […] > http://ha.ckers.org/blog/20070604/passing-malicious-php-through-getimagesize/ > > […] > > getImageSize is not better than file Info... > I didn't talked about at all. > > If the whole thing parses as an image with imagecreatefromjpeg() I > should think that's a bit tougher to create a hack that works. > maybe, but would you bet your site's security on it? > > Then one can strip off the exif info with the comments, I believe. > yeah, you just have to totally understand all of your supported media formats just to make sure that there is no other way to inject malicious php code. > > And, yes, ideally one would keep images in a totally separate > directory not even in the webtree... Which I do, but some folks can > bear the cost of passing the image "through" PHP. > yeah, for bigger sites, you would already serve the static assets through some other http server (something lightweight more suited for serving static files, like nginx, cherokee, lighty, etc.) on a separate domain, on a separate box. even if you serve those on the same vhost, you could still do engine off ( http://www.php.net/manual/en/apache.configuration.php#ini.engine) for that directory where you keep your uploaded files, which would prevent the direct execution of those files. one could still execute those malicious files if you have a Local File Inclusion vulnerability but if you do, that's game over for most cases already. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #60478 — Tom Boutell — 2012-05-05T17:59:19+00:00 URL: https://externals.io/message/60470#60478 · In reply to: #60475 This whole business of bending over backwards to prevent injection of php when apache is misconfigured just encourages apache misconfiguration IMHO. Smart people are protecting you, you don't have to do these things right, don't worry about it! Sent from my iPhone > > > […] > > getImageSize is not better than file Info... > > If the whole thing parses as an image with imagecreatefromjpeg() I > should think that's a bit tougher to create a hack that works. > > Then one can strip off the exif info with the comments, I believe. > > And, yes, ideally one would keep images in a totally separate > directory not even in the webtree... Which I do, but some folks can > bear the cost of passing the image "through" PHP. > > -- > brain cancer update: > http://richardlynch.blogspot.com/search/label/brain%20tumor > Donate: > https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=FS9NLTNEEKWBE --- ## #60479 — Sanford Whiteman — 2012-05-05T18:08:48+00:00 URL: https://externals.io/message/60470#60479 · In reply to: #60477 > > […] This presupposes that your users don't expect embedded metadata to be preserved when people redownload the images. Not only do photo professionals/hobbyists expect you to keep the metadata, you also should leave it in for reasons of legality. Hosting a bunch of stripped images can make you look really bad. We only strip metadata that is known to cause browser display problems (mostly old IE6/Adobe comment bugs). Bottom line is you have to make sure PHP never parses the files. -- S. --- ## #60480 — keisial@gmail.com — 2012-05-05T18:52:27+00:00 URL: https://externals.io/message/60470#60480 · In reply to: #60479 > This presupposes that your users don't expect embedded metadata to be > preserved when people redownload the images. > > Not only do photo professionals/hobbyists expect you to keep the > metadata, you also should leave it in for reasons of legality. Hosting > a bunch of stripped images can make you look really bad. We only strip > metadata that is known to cause browser display problems (mostly old > IE6/Adobe comment bugs). > > Bottom line is you have to make sure PHP never parses the files. > > -- S. Moreover, that still doesn't protect you, as it would be possible to make a valid image where the payload happened in the image data. I haven't tried to create such malicious image, but I have found legit images that tripped bad-image-detection heuristics looking for a 4-byte magic. Image contents are "a bunch of random bytes", but 'DROP TABLE Students;' is binary data, too. --- ## #60481 — Sanford Whiteman — 2012-05-05T22:16:23+00:00 URL: https://externals.io/message/60470#60481 · In reply to: #60480 > Moreover, that still doesn't protect you, as it would be possible to > make a valid image where the payload happened in the image data... Agreed. But sanitizing input by silently removing blocks of data your users rightfully expect to be preserved? That's egregious, even if it "worked." (Like many such discussions, I almost can't believe we're having this one... I mean, executing images is just not normal whether or not you can "bear the (performance) cost." Who is doing this on purpose?) -- S. --- ## #60484 — Paul Reinheimer — 2012-05-06T00:40:22+00:00 URL: https://externals.io/message/60470#60484 I dealt with jpegs with injected metadata quite a bit at a previous employer. In the end we ended up confirming the file was a proper image with the filetype functions, then stripping the metadata using some command line tools, and finally using a blacklist for key strings (like Or find a way to have (some of) your users have some level of trust. Or don't execute anyone's uploads. If you allow people to upload code, make them say it's code (via extension *and* by putting it in an executable area). It is not difficult to predict whether a file will be processed by PHP before worrying about what PHP would do with it. If people really worried as much as they claim to about execution of any old document, robots, htaccess, ds_stores -- and php.inis, for that matter -- would be considered highly dangerous. -- S.