Closed Bug 1368407 Opened 2 years ago Closed 2 years ago
Small PNG image hangs browser through memory consumption
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Steps to reproduce: Open the attached PNG image. Actual results: Firefox tries to eat all the memory. I let it run to >50GB on macOS, which meant the kernel started to pause other processes/applications. The image is only 41 bytes, but it defines its width as 2^32 - 1. This results in a huge malloc in libpng's png_read_start_row (pngrutil.c). Expected results: The image width should be verified or limited somehow. I believe it can be trivially fixed by defining PNG_MAX_MALLOC_64K for libpng.
Bobby, based on bug 505762 (blast from the past...), seems you might know about this code - any chance you could take a look, and/or could recommend someone else? :-)
Group: firefox-core-security → core-security
Component: Untriaged → ImageLib
Product: Firefox → Core
I stepped down from imagelib stuff years ago. You want whoever's maintaining imagelib now. I think that's tn now that seth is gone?
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is a known feature and probably doesn't need to be hidden. At one time there existed a width and height limit of 32k but that was removed a long time ago. Libpng by default uses 1,000,000 by 1,000,000 limits, but we override those (in media/libpng/pnglibconf.h) with 0x7ffffffff (2^31 -1) by 0x7fffffff limits (essentially unlimited). I don't remember the rationale for doing that. One can build with different limits by defining MOZ_PNG_MAX_WIDTH and/or MOZ_PNG_MAX_HEIGHT.
Assignee: nobody → glennrp+bmo
Status: NEW → ASSIGNED
Note that image/decoders/nsPNGDecoder.cpp also abides by user-supplied MOZ_PNG_MAX_WIDTH and MOZ_PNG_MAX_HEIGHT limits when using a system libpng instead of the in-tree libpng.
Tracking for 55, not really a new issue, but if we land a fix we could still potentially take a patch for 54.
What would you think of making MOZ_PNG_MAX_WIDTH and _HEIGHT (and maybe a new MOZ_PNG_MAX_PIXELS) settable in build:config ?
Or, more generally, MOZ_IMAGE_MAX_WIDTH, etc.?
Other places in imagelib we have used SurfaceCache::CanHold before making a potentially large allocation (even if the allocated data won't be inserted into the surface cache) to decide if we want to make the allocation to prevent making huge allocations that will likely never produce anything visible to the user.
In this case the alloc happens inside libpng where SurfaceCache::CanHold is not available. I'll see what I can do.
This patch checks for width exceeding MaximumCapacity()/8 (because libpng will allocate two row buffers of 4*width). It also checks, in the case of ADAM7 interlaced images if there is enough room to allocate the complete 4*width*height buffer that we require for deinterlacing. Please submit it to try.
On my Ubuntu-16.04 platform, I get Nightly: Attempts to allocate a huge amount of memory, then [ImgDecoder #1]: E/PNGDecoder libpng error: Out of memory Patched: Immediately abandons the PNG with this message [ImgDecoder #1]: E/PNGDecoder libpng error: Image is too wide
Sorry for the delay. https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf2da3c1a0b895657d4814e94fdcd42369e3048b
Comment on attachment 8874162 [details] [diff] [review] v00-1368407-check-png-width Try results seem ok
Attachment #8874162 - Flags: review?(tnikkel)
Attachment #8874162 - Flags: review?(tnikkel) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4142967e1f23 Check for too-large PNG width. r=tn
I reproduced this issue using Fx55.0a1 (2017-05-28) on macOS X 10.12.5. I can confirm this issue is fixed, I verified using Fx 55.0b2 on macOS X 10.12.5. Cheers!
You need to log in before you can comment on or make changes to this bug.