157202 - Exploitable (?) heap overrun in PNG

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

Subject:
exploitable heap corruption via PNG Alpha data
From:
zen-parse <[e-mail]>
Date:
Sat, 13 Jul 2002 04:04:56 +1200 (NZST)

This problem allows execution of arbitrary code via Netscape 6.2.3.

Product: 			Netscape 6.x
Platform: 			Unix: x86 Linux
Operating system version: 	Redhat 7.0

exploitable heap corruption via PNG Alpha data 
I believe the fault lies somewhere around here:
http://lxr.mozilla.org/mozilla/source/gfx2/src/gfxImageFrame.cpp#326
A specially crafted invalid PNG, with reported dimensions of 65536x65536
such as and an 8-bit alpha channel (most likely, similar overflowing
values as well, such as 65531x32771 with 16-bit alpha, if that is
supported),can cause controllable heap corruption, overwriting (at least)
a pointer to a function pointer that allows arbitary code to be executed.

An example exploit for this is on:

http://crash.ihug.co.nz/~Sneuro/pngkiller2.tar.gz

The code is somewhat commented and explains what I believe is happening,
although it was written while offline, and so without access to the source
to verify the accuracy.

However, this is a DIFFERENT problem to the previously reported PNG
problem.

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Adding keywords, this sounds quite serious. Could someone (pavlov?) confirm this
crash?

Comment 2

[Stuart Parmenter]

I wasn't seeing the crash on my solaris box, but I'll try it on a few other 
machines.  I am having trouble building the crash code due to missing libraries 
and headers.

Comment 3

[Mitchell Stoltz (not reading bugmail)]

From Georgi:

Bug 157202 Exploitable (?) heap overrun in PNG really seems exploitable.
I had problems running the programs (probably because they need a lot of
disk space).
But the supplied laa.png really crashes mozilla in a nasty way - writes 0x42 to
the stack.

Georgi

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Also, does this cause a crash on Windows?

Comment 5

[Stuart Parmenter]

where did Georgi find laa.png?

Comment 6

[tor]

Two problems at first glance:

  * nsImageGTK uses signed shorts to describe alpha data metrics.  Changing
    them in nsImageGTK.h to PRUInt32s should eliminate that problem.

  * mozilla doesn't sanity check the amount of memory requested for an image.
    The example file (laa.png) wants 65536*65536*4 bytes.

Along the lines of excessive memory use, I'm not aware that we have any limit
to the total amount of memory used by the image cache for a page.  A possible
DOS attack would be to create a page with a number of large dimension solid
block pngs (which compress into trivially small files).

Comment 7

[Stuart Parmenter]

Attachment: change int16s to int32s

this should fix the overflow on linux.	the overflow doesn't occur on the other
platforms.

Comment 8

[georgi - hopefully not receiving bugspam]

Stuart Parmenter: the file laa.png is in the tar pngkiller2.tar.gz

Comment 9

[Jaime Rodriguez, Jr.]

Removing adt1.0.1, until this has been reviewed and verified on the trunk. 

Who can review and supper review Pav's patch? 

Comment 10

[Stuart Parmenter]

dougt: can you r=?  tor: can you sr=?

Comment 11

[Doug Turner (:dougt)]

Comment on attachment 91785 [details] [diff] [review]
change int16s to int32s

do we have any logic that depends on the size of these ?

if not and assuming that this is well tested, r=dougt

Comment 12

[tor]

Comment on attachment 91785 [details] [diff] [review]
change int16s to int32s

sr=tor

There's some logic in the alpha compositing that will break down if
screen dimensions get bigger than 16 bits, but since the X protocol
will break first we're safe.

Comment 13

[Jaime Rodriguez, Jr.]

Pav - Looks like you have the reviews. Can you pls land this on the trunk
tonight, so that we can have it verified by bindu tomorrow? thanks!

Comment 14

[chris hofmann]

Comment on attachment 91785 [details] [diff] [review]
change int16s to int32s

a=chofmann for trunk/1.1b

Comment 15

[Stuart Parmenter]

landed on trunk.

Comment 16

[scottputterman]

Bindu, can you verify this on the trunk?

Comment 17

[georgi - hopefully not receiving bugspam]

I disagree with Comment #7 "the overflow doesn't occur on the other
platforms."
The file laa.png crashes moz1.0 on win2k.
Are you sure the fix in *gtk* also fixes win2k?

Comment 18

[georgi - hopefully not receiving bugspam]

Attachment: This crashes on linux even after the proposed patch

Attached png consumes even more memory and still crashes latest CVS build on at
least linux.

Comment 19

[georgi - hopefully not receiving bugspam]

I think this bug should be reopened per my 2 previous comments.
Today's linux CVS build passes the crash with laa.png (the reporters testcase)
but if it  is modified a little mozilla still crashes with the attached to this
bug testcase.
http://bugzilla.mozilla.org/attachment.cgi?id=91955&action=view

Comment 20

[tor]

As I mentioned in comment 6, the nsImageGTK change is only a small portion
of the problem.

Comment 21

[georgi - hopefully not receiving bugspam]

My testcase seems to abort() mozilla and doesn't seem exploitable on linux IMHO.
The problem is a signed/unsigned issue.

Comment 22

[bsharma]

Verified on 2002-07-19-trunk build on Win 2K and Linux.

Try to open the laa.png in http://crash.ihug.co.nz/~Sneuro/pngkiller2.tar.gz
and the Win 2K and Linux crashes.

Comment 23

[Stuart Parmenter]

Attachment: png decoder patch to pay attention to errors when initing the gfxImageFrame

Comment 24

[Stuart Parmenter]

Attachment: limit size of images to 2^16 x 2^16

Comment 25

[Stuart Parmenter]

Attachment: jpeg and mng patches to pay attention to error

Comment 26

[Stuart Parmenter]

Attachment: jpeg and mng patches to pay attention to the error

Comment 27

[Stuart Parmenter]

Attachment: limit size of images to 2^14 x 2^14

Comment 28

[Stuart Parmenter]

Attachment: fix ppm and xbm decoders as well

Comment 29

[Stuart Parmenter]

Attachment: one patch for all the decoders

Comment 30

[Stuart Parmenter]

ok, these 2 patches limit image sizes to 2^14 (16k) and will error out if 
images try to get bigger than that.

Comment 31

[tor]

Comment on attachment 92024 [details] [diff] [review]
one patch for all the decoders

sr=tor

Comment 32

[Doug Turner (:dougt)]

Comment on attachment 92024 [details] [diff] [review]
one patch for all the decoders

r=dougt

Comment 33

[saari (gone)]

Comment on attachment 92021 [details] [diff] [review]
limit size of images to 2^14 x 2^14

r=saari

Comment 34

[Stuart Parmenter]

Attachment: limit size to (2^15)-1

Comment 35

[tor]

Comment on attachment 92030 [details] [diff] [review]
limit size to (2^15)-1

sr=tor

Comment 36

[Doug Turner (:dougt)]

Comment on attachment 92030 [details] [diff] [review]
limit size to (2^15)-1

r=dougt

Comment 37

[Asa Dotzler [:asa]]

Comment on attachment 92024 [details] [diff] [review]
one patch for all the decoders

a=asa (on behalf of drivers) for checkin to 1.1

Comment 38

[Asa Dotzler [:asa]]

Comment on attachment 92030 [details] [diff] [review]
limit size to (2^15)-1

a=asa (on behalf of drivers) for checkin to 1.1

Comment 39

[Stuart Parmenter]

fixes checked in.

Comment 40

[Jaime Rodriguez, Jr.]

bsharma: bindu, can you verify this fix on the trunk? thanks!

Comment 41

[bsharma]

Verified on 2002-07-22-trunk on Win 2000.

The laa.png does not load and the error appears in the content window.

Comment 42

[Paul Wyskoczka]

Adding adt1.0.1 on behalf of the adt.  Please check this fix into the Mozilla
branch after getting branch approval from drivers.

Comment 43

[chris hofmann]

a=chofmann for 1.0.1.  add the fixed1.0.1 keyword after checking in on the branch.

Comment 44

[Jaime Rodriguez, Jr.]

bsharma: bindu, can you verify this fix on the branch, then replace "fixed1.0.1"
with "verified1.0.1"? thanks!

Comment 45

[bsharma]

Verified on 2002-07-25-branch on Win 2000.

The laa.png does not load and the error appears in the content window.

Comment 46

[tor]

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