Decompression bomb protection

[masklinn]

"zip bombs" are a somewhat know threat, but it also applies to images and can't be protected against by checking the filesystem size of the data:

• http://www.aerasec.de/security/advisories/decompression-bomb-vulnerability.html provides 2 PNG bomb demos, a 4KB 6000x6000 images decompressing to 100MB in memory and a 44KB 19000x19000 image decompressing to 1GB
• JPEG is somewhat less efficient, but e.g. NASA Visible Earth's Blue Marble has 21600x21600 tiles as small as 5MB (tile A2, southern pacific) decompressing to 1.4GB

This means it's possible to DOS e.g. a web application performing image resizing by sending one of these bombs. As far as I can tell the protection possibilities are limited:

• assert source image sizes before doing any operation which will need the image data, the documentation may benefit from a warning on that subject (similar to warnings about XML vulnerabilities in the Python documentation), Image.open could be augmented with e.g. a maximum_pixels parameter raising an error in case image.h * image.w goes above the specified limit to make this easier for users
• during image loading/decompression, raise an error if the decompressed data size gets above a specific threshold

[wiredfool]

I suspect that a nearly empty image that only included size metadata and enough plausible data to start the decoder would trigger a DOS. Essentially, Image.new allocates the storage before passing it off to the decoder to fill. And that's solely based on the size and the number of bands.

There's now protection in the BMP decoder against that, but the limit is set rather high, 2GP or something like that.

I can see adding a limit to the allocator that would raise an exception if a memory limit was exceeded. I'm not 100% sure that I want to enable it by default -- I suspect that the scientific imaging community would be the ones that would bump up against it in the course of ordinary work.

[d-schmidt]

Big jpegs aren't even a problem if you are using thumbnail(). But we did test our backend with image like Carina Nebula http://hubblesite.org/gallery/album/entire/pr2007016a/warn/
The pngs are interesting in a multi processing environment, Going to run some tests with these.

edit: another thing to note is that Image.open() will not trigger the bombs. Images aren't really loaded until you actually do something with the image.

im = Image.open('evil.png')
if im.size[0] * im.size[1] > arbitary_large_limit:
    raise ImageIsToBigError("image size exceeds limit")
im.do_something()

Should solve this bomb problems just fine.

[masklinn]

Big jpegs aren't even a problem if you are using thumbnail().

Interesting. The issue still exists for other operations though right? And for non-JPEG plugins, unless the author has specifically taken steps to avoid it?

How comes thumbnail works but resize seems to load the whole image in memory? Tricks in the JPEG image plugin?

another thing to note is that Image.open() will not trigger the bombs.

Yes I didn't specifically note it, as the doc states sufficiently clearly that Image.open is a lazy operation and does not decode&load pixel data until they're needed.

Should solve this bomb problems just fine.

It "does", if you know about the issue in the first place. Hence my suggestion to add a warning on the subject in Pillow's documentation, similar to the standard library warning about entity-based attacks on XML parsers. I think that'd be a baseline solution, though I also think protecting users by default (by adding an overridable check directly in Image.open) would be a good idea.

[wiredfool]

JPEG thumbnails are a special case because of the way that JPEG compression works. It's a spatial frequency approach, rather than a raster line compression. If you cut off the frequency low enough, you don't have to read most of the data.

[aclark4life]

Where is the PR for this issue, assuming one exists?

[aclark4life]

Anyone have any interest in working on this for 2.5.0?

[hugovk]

For starters, how about something along the lines of this?
hugovk@ce2955e#diff-3fef5f960d5bd8bdfb9c6fadcf96af22L2100

Image.open() takes a new maximum_pixels parameter, defaulting to some predetermined ARBITARY_LARGE_LIMIT (6000 * 6000 - 1 for testing).

If the image is larger than this, throw an ImageIsTooBigError exception.

This test code:

from PIL import Image

print(1, "No exception:")
im1 = Image.open("picture-100M-6000x6000.png", maximum_pixels=None)

print(2, "No exception:")
im2 = Image.open("picture-100M-6000x6000.png", maximum_pixels=6000*6000)

print(3, "Exception:")
im3 = Image.open("picture-100M-6000x6000.png")

print(4, "End")

Outputs:

(1, 'No exception:')
(2, 'No exception:')
Pixels: 36000000
(3, 'Exception:')
Pixels: 36000000
Traceback (most recent call last):
  File "C:\stufftodelete\test.py", line 10, in <module>
    im3 = Image.open("picture-100M-6000x6000.png")
  File "C:\Python27\lib\site-packages\PIL\Image.py", line 2128, in open
    _compression_bomb_check(im, maximum_pixels)
  File "C:\Python27\lib\site-packages\PIL\Image.py", line 2085, in _compression_bomb_check
    raise ImageIsTooBigError("Image size exceeds limit")
PIL.Image.ImageIsTooBigError: Image size exceeds limit

[hugovk]

See #674.

[d-schmidt]

This would break existing code. I'm all in for an optional parameter, but we can't just throw a new Exception now.

Nokia lumia pictures are already bigger than 36M.

[xmo-odoo]

I'm all in for an optional parameter

That would be no improvement over the current situation.

[d-schmidt]

It would be, you could block giant images if you want to. As far as I understand these, we are talking about simple two colored png images. Which will take a lot of memory if you load them, but that isn't really a bug and the arbitary low limit of 36M would break existing code for no urgent reason.

IMHO API changes as intrusive as this should only happen for a very good reason.

[wiredfool]

My thought process, which is in short bits at the moment.

• As noted above, it's currently possible to block large images entirely outside the library, if desired, since open is a lazy operation.
• What is a useful image and what is a DOS is a matter of implementation and deployment. We have support for > 2Gpx images, and someone was recently asking for support for ~4GB tiffs.
• I don't think that throwing an exception is a good move, without the programmer requesting this.
• Documentation is good. In general.

So, I can see a couple of options.

• If we use exceptions, I'd default to having no limit, and an easy drop in limit if an exception is desired. e.g. Image.MAX_IMAGE_PIXELS=None by default, Image.MAX_IMAGE_PIXELS=whatever to enable DOS protection.
• We can issue a warning, and have documentation for how to turn that into an exception.

[masklinn]

you could block giant images if you want to.

Which you can already do "by hand" if you know and remember the problem exists. Having to opt-in via an optional parameter isn't an improvement over having to opt-in via an assert or a one-line conditional.

As far as I understand these, we are talking about simple two colored png images.

1. the images in the original comment are examples and demonstration of how bad things can get
2. NASA's blue marble set is "a true color earth dataset including seasonal dynamics from MODIS", it's neither simple nor two-colored (and it's real-world data, selected specifically to preclude accusations of artificialness and non-representativity) (not that this matters to somebody trying to DOS your service)

Which will take a lot of memory if you load them, but that isn't really a bug

A bug's in the eye of the beholder, decompression bombs allow DOSing services using PIL unless the developer was aware of the problem (which is not documented anywhere near Image.open) and took explicit steps to prevent it (or lucked out and only performs operations which don't trigger loading image data in full) and any service which uses pillow and happens to trigger a full image load is broken by default.

We can issue a warning, and have documentation for how to turn that into an exception.

An issue here being the ability to disable the warning without breaking previous pillow versions.

[wiredfool]

Warnings can be ignored or turned into exceptions at the interpreter level, so they're safe to add from the POV of breaking existing code.

[masklinn]

True.

[hugovk]

Thanks for all the comments.

So how about this?

1. Trigger a warning if the image is over a certain limit, Image.MAX_IMAGE_PIXELS.
2. Image.MAX_IMAGE_PIXELS is set to some sensible default (what?), but is changeable.
3. Documentation about possible DOS attacks and the changeable limit, including instructions on how to turn the warning into an exception if desired.

[hugovk]

if so, see updated #674.

Warning text and limit need checking.

This test code:

from PIL import Image

print(1, "Warning:")
print Image.MAX_IMAGE_PIXELS
im1 = Image.open("picture-100M-6000x6000.png")

print(2, "No warning:")
Image.MAX_IMAGE_PIXELS = None
print Image.MAX_IMAGE_PIXELS
im2 = Image.open("picture-100M-6000x6000.png")

print(3, "No warning:")
Image.MAX_IMAGE_PIXELS = 6000*6000
print Image.MAX_IMAGE_PIXELS
im3 = Image.open("picture-100M-6000x6000.png")

print(4, "Warning:")
Image.MAX_IMAGE_PIXELS = 10
print Image.MAX_IMAGE_PIXELS
im4 = Image.open("picture-100M-6000x6000.png")

print(5, "Done")

Outputs:

(1, 'Warning:')
35999999
c:\Python27\lib\site-packages\PIL\Image.py:2084: RuntimeWarning: Image size (36000000 pixels) exceed
s limit of 35999999 pixels, could be DOS attack.
  RuntimeWarning
(2, 'No warning:')
None
(3, 'No warning:')
36000000
(4, 'Warning:')
10
c:\Python27\lib\site-packages\PIL\Image.py:2084: RuntimeWarning: Image size (36000000 pixels) exceed
s limit of 10 pixels, could be DOS attack.
  RuntimeWarning
(5, 'Done')

[masklinn]

Image.MAX_IMAGE_PIXELS is set to some sensible default (what?), but is changeable.

For cell phones, the biggest resolutions are Nokia's Lumia 808 and 1020 with 41.3MP sensors and 38.2MP maximum resolution (in 4:3 aspect ratio, 33.6MP in 16:9) (note: the default resolution for the 1020 is 5MP).

For SLR, looking at dpreview.com they're in the 24~36Mp range, except for a few medium-format $10k and more 50MP cameras (the Phase One IQ2 at ~$40k, the Hasselblad H5D-50c at ~$30k and the Pentax 645Z announced at $8500)

[hugovk]

The default for the Nokia 808 (which isn't a Lumia) is also 5 MP.

[wiredfool]

I'd lean toward setting the default warning level rather high, perhaps on the level of 1/4-1/2 gig of memory. It's big enough that anyone intentionally using a larger image would likely know that they're doing something that might require reading the documentation, and it's small enough that it's not going to exhaust most servers (in the event that you get only one of them, anyway).

Alternately, one could assume that it's a big enough image to be annoying and it's small enough to process in my default dev vms.

[hugovk]

PR #674 updated with tests, further feedback welcome.

TODO:

• Check warning text.
• Documentation.

[hugovk]

@masklinn @wiredfool @d-schmidt @xmo-odoo Any comments on the current state of PR #674?

[aclark4life]

@masklinn @wiredfool @d-schmidt @xmo-odoo @hugovk FYI deadline approaching 🐎

[masklinn]

@aclark4life commented on #674, that seemed smarter.

[hugovk]

#674 has now been merged into master.

Documentation was suggested earlier. How about something like this? Feel free to completely rewrite it.

To protect against potential DOS attacks caused by "decompression bombs" (i.e. malicious files which decompress into a huge amount of data and are designed to crash or cause disruption by using up a lot of memory), Pillow will issue a DecompressionBombWarning if the image is over a certain limit. If desired, the warning can be turned into an exception by ...[HOW?]...

And where should this go in the docs?

[masklinn]

A warning box in the Image.open text? The final part can probably just link to the warnings filter documentation. Either that, or give an example e.g. warnings.simplefilter('error', Image.DecompressionBombWarning) to make it into an error, warnings.simplefilter('ignore', Image.DecompressionBombWarning) to suppress it entirely, and maybe a note/link to the logging documentation to have warnings output to the logging facility instead of stderr.

[aclark4life]

Can we close this?

[hugovk]

@aclark4life Let's keep it open until the docs are updated. I'll make a PR.

[hugovk]

@aclark4life Please can you check the text is ok and the markup is correct in #738?

[hugovk]

OK, docs updated. Let's fix them up as needed later. I'll close this now. Thanks for all the input!