Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Denial of Service Vulnerability With Maliciously Crafted JPEGs #1513

Closed
pbyrne opened this issue Apr 10, 2014 · 10 comments · Fixed by #1514
Closed

Denial of Service Vulnerability With Maliciously Crafted JPEGs #1513

pbyrne opened this issue Apr 10, 2014 · 10 comments · Fixed by #1514

Comments

@pbyrne
Copy link
Contributor

pbyrne commented Apr 10, 2014

Originally reported at https://hackerone.com/reports/390.

A specially crafted JPEG (lottapixel.jpg) causes attempts to determine the dimensions of the image to exhaust available memory. From the original report, linked above:

The exploit is really simple. I have an image of 5kb, 260x260 pixels. In the image itself I exchange the 260x260 values with 0xfafa x 0xfafa (so 64250x64250 pixels). Now from what I remember your service tries to convert the image once uploaded. By loading the 'whole image' into memory, it tries to allocate 4128062500 pixels into memory, flooding the memory and causing DoS.

HackerOne solved this in their app by monkey-patching paperclip's GeometryDetector#geometry_string method, a solution which we've also adopted. By switching the identify -format string from %wx%h,%[exif:orientation] to %wx%h,1, the problem goes away at the cost of losing the ability to use that EXIF data to determine orientation.

@pbyrne
Copy link
Contributor Author

pbyrne commented Apr 10, 2014

I'm not familiar enough with ImageMagick to know whether it's possible to still retrieve that EXIF data without making yourself vulnerable to this attack. A configuration option to disable loading the EXIF data would allow anyone who needs to use it to continue to do so, while those who don't can prevent this problem. Would you be interested in receiving a pull request that implements this option?

@jyurek
Copy link

jyurek commented Apr 10, 2014

Yes, I would be interested in that. Thank you for reporting this.

@pbyrne
Copy link
Contributor Author

pbyrne commented Apr 11, 2014

Awesome. Here you go: #1514

Note, I've been getting some random and inconsistent Travis failures on my branch that don't seem to be related to my change. I've triggered another build, so hopefully this time it's green.

@jjoos
Copy link

jjoos commented Jun 17, 2014

Thank @pbyrne for submitting this.

Sorry for not submitting this patch myself...

@christineyen
Copy link

It seems like specifying a :styles: will still trigger the image being read into memory - because there's no way (?) to validate the dimensions of an image before the processors run, the Thumbnail processor still tries to read + convert the image.

I had to add a separate before_post_process check to abort if the image's dimensions were beyond a certain size. Should this be a different issue / patch, or is there a cleaner way to get around this issue?

@groe
Copy link
Contributor

groe commented Jun 16, 2015

@pbyrne @sikachu Am I assuming right that the default for every paperclip app is to be vulnerable to this exploit?

I can report that the exploit is actively being used already (and it's a very effective way to make every server go down within seconds). One of my sites has been attacked today and it's shocking to see a quite powerful machine going down within seconds just for one 5kb JPG file being uploaded by anyone.

@pbyrne
Copy link
Contributor Author

pbyrne commented Jun 16, 2015

I believe the default behavior of Paperclip is still to try to respect the EXIF data when determining dimensions. Whether the latest version of ImageMagick is still susceptible to this vulnerability, I'm not certain.

@groe
Copy link
Contributor

groe commented Jun 16, 2015

I am on ubuntu 14.04 and the imagemagick version from the apt repositories (ImageMagick 6.7.7-10 2014-03-06 Q16) is still vulnerable.

Will this be fixed in imagemagick at all? Or will client libraries need to handle it before passing such an image to imagemagick?

@groe
Copy link
Contributor

groe commented Jun 17, 2015

Ok the correct solution to this should be to define a policy.xml file for imagemagick. See here.

@jjoos
Copy link

jjoos commented Aug 8, 2015

@groe I tried fixing some issue with the policy.xml, but some of those limits (processing time) aren't checked very regularly. It was some time ago, but I remember I had a limit of 3 seconds and the process would quite after 30 seconds saying it exceeded the limit of 3 seconds.

I ended up creating wrappers for Imagemagick executables like convert:

#!/usr/bin/env zsh

limit -h cputime 4

/usr/local/bin/convert "$@"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants