Closed Bug 155222 Opened 18 years ago Closed 18 years ago
Heap corruption in PNG library
Submitter name: zen-parse Submitter email address: firstname.lastname@example.org Acknowledgement checkbox: on Product: Communicator 4.x Operating system: Unix: x86 Linux OS version: Redhat 7.0 Issue summary: PNG files can cause potentially exploitable heap corruption Issue details: Heap Corruption Vulnerability. It is possible to cause heap corruption in Netscape and Mozilla by supplying a specially crafted PNG format file. If the X size > (2^32 / number_bytes_needed_per_pixel) then the number of bytes required for a row becomes greater than 2^32 and overflows. This condition is checked for in the png library, but by default isn't treated as fatal (to the image) in Mozilla/Netscape. (warning handler) pngtopnm (from netpbm-progs package) complains about the file with: [zen@clarity netscaper]$ pngtopnm laa.png libpng warning: Width too large to process image data; rowbytes will overflow. pnmtopng: fatal libpng error: Extra compressed data pngtopnm: setjmp returns error condition (... Just found a variation on it. A much larger IDAT chunk than the data size is also causes corruption. I'm not sure of the exact mechanism for that yet. [zen@clarity netscaper]$ pngtopnm lbt.png pnmtopng: fatal libpng error: Extra compressed data pngtopnm: setjmp returns error condition ...) [zen@clarity netscaper]$ file laa.png laa.png: PNG image data, 1073741952 x 2, 8-bit/color RGBA, non-interlaced [zen@clarity netscaper]$ netscape laa.png (netscape opens, then closes when I move the mouse) Bus error [zen@clarity netscaper]$ gdb /usr/lib/netscape/netscape-communicator GNU gdb Red Hat Linux (5.1-0.7) Copyright 2001 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "i386-redhat-linux"... (no debugging symbols found)... (gdb) q [zen@clarity netscaper]$ gdb -q /usr/lib/netscape/netscape-communicator (no debugging symbols found)...(gdb) r laa.png Starting program: /usr/lib/netscape/netscape-communicator laa.png Program received signal SIGSEGV, Segmentation fault. 0x4005f510 in _XtGetModifierIndex () from /usr/X11R6/lib/libXt.so.6 (gdb) x/i $eip 0x4005f510 <_XtGetModifierIndex+576>: movzwl 0x2(%edi),%edx (gdb) info reg edi edi 0xdeadbeef -559038737 (gdb) c Continuing. Program received signal SIGBUS, Bus error. 0x40229bb1 in __kill () from /lib/libc.so.6 (gdb) c Continuing. Program terminated with signal SIGBUS, Bus error. The program no longer exists. (gdb) q [zen@clarity netscaper]$ The problem is potentially exploitable to allow running arbitrary code, but due to the asyncronous way events occur, it is not trivial to achieve this. I have managed to get eip=0xdeadbeef on a number of occasions, but the same file has also given other random errors on further runs depending on the order of events processed. Also making one slight change to the file can massively change the location of things in memory. Exploitation is (at least theoretically) possible though. A potential fix would be to enable a warning function handler in the png libraries. The check in http://lxr.mozilla.org/mozilla/source/modules/libimg/png/pngget.c#569 is the appropriate file and http://crash.ihug.co.nz/~Sneuro/netscape-example.png is an example of a corrupt file. (Just the latest one I was playing with.) This type of problem also affects Opera 6.0 (so don't feel so bad). Additional computer info: The gdb dump is from 4.77 (not the latest version) but similar problems exist in later versions (according to reports of crashes from friends who tried to view the file.) This form was submitted from http://help.netscape.com/forms/bug-security.html?cp=bbpctr with Mozilla/4.76 (Linux 2.2.19-7.0.16 i686; U) Opera 6.0 [en].
Some quick tests show that gimp and konqueror are also vulnerable on linux. openoffice on windows also becomes unresponsive.
Even though this is my whack at the patch, I strongly suspect we should also do something about the return value (which seems to be ignored currently at http://lxr.mozilla.org/mozilla/source/modules/libpr0n/decoders/png/nsPNGDecoder.cpp#179 ) Somebody with more png experience should look at this as well.
Comment on attachment 89975 [details] [diff] [review] patch that changes that warning to an error. while we probably should add a check in the info_callback code so that we listen to the return value of our call to png_get_IHDR, changing the warning to an error will result in a longjmp out of the code and back to "safe" land. it will then abort loading of the image and all should be good.
Attachment #89975 - Flags: review+
Glenn, does it sound reasonable that this condition should call png_error() instead of png_warning() inside libpng, since similar tests in png_get_IHDR() also call png_error()?
Comment on attachment 89975 [details] [diff] [review] patch that changes that warning to an error. The libpng style in png_get_IHDR() is to just call png_error() without a change in the return value. Also add a line to MOZCHANGES so that we can keep track of mozilla changes.
At first glance it looks reasonable to change the warning to an error. I'll think about it more when I get back home tonight. Glenn
After further thought I still agree with changing the png_warning() to png_error(). In libpng-1.0.13 and libpng-1.2.4 we have added a small safety margin (see malloc of png_ptr->big_row_buf in pngrutil.c of libpng-1.2.x), and the test should be if ((*width > PNG_MAX_UINT/rowbytes_per_pixel) - 64) instead of if ((*width > PNG_MAX_UINT/rowbytes_per_pixel)) To avoid having a (pathological) class of images that can be processed with mozilla+libpng-1.0.9+ and not with mozilla+libpng-1.2.x, I suggest that the same test be used now in the patch to libpng-1.0.9. I'll leave it to tor to update the attachment. Glenn
I wrote: >the test should be > > if ((*width > PNG_MAX_UINT/rowbytes_per_pixel) - 64) > > >instead of > > if ((*width > PNG_MAX_UINT/rowbytes_per_pixel)) Phooey, misplaced ")". Anyhow there seems to be an unnecessary set of (). The test should be if (*width > PNG_MAX_UINT/rowbytes_per_pixel - 64) Glenn
>Just found a variation on it. A much larger >IDAT chunk than the data size is >also causes corruption. I'm not sure of the >exact mechanism for that yet. This part of the original bug report appears to duplicate bug <a href="http://bugzilla.mozilla.org/show_bug.cgi?id=154996">154996</a> which is being fixed. Glenn
tor? is this patch ok?
Comment on attachment 90861 [details] [diff] [review] as above with Glenn's suggested change r=pavlov
Attachment #90861 - Flags: review+
tor, could you do an sr= for this?
Comment on attachment 90861 [details] [diff] [review] as above with Glenn's suggested change sr=tor
Attachment #90861 - Flags: superreview+
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
Comment on attachment 90861 [details] [diff] [review] as above with Glenn's suggested change a=asa (on behalf of drivers) for checkin to the 1.1 trunk.
Attachment #90861 - Flags: approval+
fixed on trunk. need adt approval for branch
Bindu, can you verify this fix on the trunk. If you need help in testing, please contact Gagan not Mitch.
nope. mitch would be right contact for help with this, not me.
Gagan, do we have a testcase that causes this crash, something Bindu can click on and run?
none that i know of. ask the reporter?
http://crash.ihug.co.nz/~Sneuro/testing123.png PNG image data, 536879104 x 1, 8-bit/color RGB, non-interlaced http://crash.ihug.co.nz/~Sneuro/testing234.png PNG image data, 1073741825 x 1, 8-bit/color RGB, non-interlaced http://crash.ihug.co.nz/~Sneuro/testing345.png PNG image data, 1431655770 x 1, 8-bit/color RGB, non-interlaced
Verified on 2002-07-16-trunk build on Win 2K. Loaded following test png files: http://crash.ihug.co.nz/~Sneuro/netscape-example.png http://crash.ihug.co.nz/~Sneuro/testing123.png http://crash.ihug.co.nz/~Sneuro/testing234.png http://crash.ihug.co.nz/~Sneuro/testing345.png All of above test cases gives the following error in the browser window: The image “http://crash.ihug.co.nz/~Sneuro/<png file name>.png” cannot be displayed, because it contains errors.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
adding adt1.0.1+. Please get drivers approval before checking into the branch.
changing to verified on trunk based on Bindu's comments.
Status: RESOLVED → VERIFIED
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+" keyword and add the "fixed1.0.1" keyword.
Verified on 2002-07-19-branch build on Win 2K. Loaded following test png files: http://crash.ihug.co.nz/~Sneuro/netscape-example.png http://crash.ihug.co.nz/~Sneuro/testing123.png http://crash.ihug.co.nz/~Sneuro/testing234.png http://crash.ihug.co.nz/~Sneuro/testing345.png All of above test cases gives the following error in the browser window: The image “http://crash.ihug.co.nz/~Sneuro/<png file name>.png” cannot be displayed, because it contains errors.
Just for pedantry's sake shouldn't OS be set to All?
Since this bug was discussed publicly in the libpng mailing lists and is described and fixed publicly in libpng-1.2.4/1.0.14, perhaps it can be made a "public" Mozilla bug. Incidentally, bug #154996 is public, and the security implications of that bug are quite similar. PS: I agree that changing OS and platform to "ALL" would make sense, but I'm not sufficiently empowered to do that. Glenn
*** Bug 161086 has been marked as a duplicate of this bug. ***
OS: Windows 2000 → All
Hardware: PC → All
You need to log in before you can comment on or make changes to this bug.