Closed Bug 155222 Opened 18 years ago Closed 18 years ago

Heap corruption in PNG library

Categories

(Core :: Security, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.0.1

People

(Reporter: security-bugs, Assigned: pavlov)

References

Details

(Whiteboard: [adt1 rtm])

Attachments

(1 file, 2 obsolete files)

Submitter name:                zen-parse
   Submitter email address:       zen-parse@gmx.net
   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.
ccing tor
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
Attached patch updated patch with tor's review (obsolete) — Splinter Review
Attachment #89975 - Attachment is obsolete: true
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?
Attachment #90046 - Attachment is obsolete: true
Whiteboard: [adt1 rtm]
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.
Keywords: adt1.0.1adt1.0.1+
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.
checked in on branch
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.

Added the verified1.0.1 keyword.
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. ***
Group: security?
OS: Windows 2000 → All
Hardware: PC → All
You need to log in before you can comment on or make changes to this bug.