155222 - Heap corruption in PNG library

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

Submitter name:                zen-parse
   Submitter email address:       [e-mail]
   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].

Comment 1

[georgi - hopefully not receiving bugspam]

Some quick tests show that gimp and konqueror are also vulnerable on linux.
openoffice on windows also becomes unresponsive.

Comment 2

[Gagan]

Attachment: patch that changes that warning to an error.

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 3

[Gagan]

ccing tor

Comment 4

[Stuart Parmenter]

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.

Comment 5

[tor]

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 6

[tor]

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.

Comment 7

[Glenn Randers-Pehrson]

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

Comment 8

[Gagan]

Attachment: updated patch with tor's review

Comment 9

[Glenn Randers-Pehrson]

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

Comment 10

[Glenn Randers-Pehrson]

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

Comment 11

[Glenn Randers-Pehrson]

>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

Comment 12

[Stuart Parmenter]

tor?  is this patch ok?

Comment 13

[tor]

Attachment: as above with Glenn's suggested change

Comment 14

[Stuart Parmenter]

Comment on attachment 90861 [details] [diff] [review]
as above with Glenn's suggested change

r=pavlov

Comment 15

[scottputterman]

tor, could you do an sr= for this?

Comment 16

[tor]

Comment on attachment 90861 [details] [diff] [review]
as above with Glenn's suggested change

sr=tor

Comment 17

[Asa Dotzler [:asa]]

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.

Comment 18

[Stuart Parmenter]

fixed on trunk.  need adt approval for branch

Comment 19

[Paul Wyskoczka]

Bindu, can you verify this fix on the trunk.  If you need help in testing,
please contact Gagan not Mitch.

Comment 20

[Gagan]

nope. mitch would be right contact for help with this, not me. 

Comment 21

[Mitchell Stoltz (not reading bugmail)]

Gagan, do we have a testcase that causes this crash, something Bindu can click
on and run?

Comment 22

[Gagan]

none that i know of. ask the reporter? 

Comment 23

[neuro]

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

Comment 24

[bsharma]

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.

Comment 25

[scottputterman]

adding adt1.0.1+. Please get drivers approval before checking into the branch.

Comment 26

[scottputterman]

changing to verified on trunk based on Bindu's comments.

Comment 27

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 28

[Stuart Parmenter]

checked in on branch

Comment 29

[bsharma]

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.

Comment 30

[bsharma]

Added the verified1.0.1 keyword.

Comment 31

[Simon Lucy]

Just for pedantry's sake shouldn't OS be set to All?

Comment 32

[Glenn Randers-Pehrson]

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

Comment 33

[tor]

*** Bug 161086 has been marked as a duplicate of this bug. ***