Heap corruption in PNG library

VERIFIED FIXED in mozilla1.0.1

Status

()

Core
Security
VERIFIED FIXED
15 years ago
15 years ago

People

(Reporter: Mitchell Stoltz (not reading bugmail), Assigned: Stuart Parmenter)

Tracking

Trunk
mozilla1.0.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt1 rtm])

Attachments

(1 attachment, 2 obsolete attachments)

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.

Comment 2

15 years ago
Created attachment 89975 [details] [diff] [review]
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

15 years ago
ccing tor
(Assignee)

Comment 4

15 years ago
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+

Comment 5

15 years ago
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

15 years ago
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

15 years ago
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

15 years ago
Created attachment 90046 [details] [diff] [review]
updated patch with tor's review

Updated

15 years ago
Attachment #89975 - Attachment is obsolete: true

Comment 9

15 years ago
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
(Assignee)

Comment 12

15 years ago
tor?  is this patch ok?

Comment 13

15 years ago
Created attachment 90861 [details] [diff] [review]
as above with Glenn's suggested change
Attachment #90046 - Attachment is obsolete: true
(Reporter)

Updated

15 years ago
Keywords: adt1.0.1, mozilla1.0.1, nsbeta1
Whiteboard: [adt1 rtm]
(Assignee)

Comment 14

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

r=pavlov
Attachment #90861 - Flags: review+

Comment 15

15 years ago
tor, could you do an sr= for this?

Comment 16

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

sr=tor
Attachment #90861 - Flags: superreview+
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1

Comment 17

15 years ago
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+
(Assignee)

Comment 18

15 years ago
fixed on trunk.  need adt approval for branch

Comment 19

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

Comment 20

15 years ago
nope. mitch would be right contact for help with this, not me. 
(Reporter)

Comment 21

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

Comment 22

15 years ago
none that i know of. ask the reporter? 

Comment 23

15 years ago
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

15 years ago
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
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 25

15 years ago
adding adt1.0.1+. Please get drivers approval before checking into the branch.
Keywords: adt1.0.1 → adt1.0.1+

Comment 26

15 years ago
changing to verified on trunk based on Bindu's comments.
Status: RESOLVED → VERIFIED

Comment 27

15 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
(Assignee)

Comment 28

15 years ago
checked in on branch
Keywords: mozilla1.0.1+ → fixed1.0.1

Comment 29

15 years ago
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

15 years ago
Added the verified1.0.1 keyword.
Keywords: fixed1.0.1 → verified1.0.1

Comment 31

15 years ago
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

Comment 33

15 years ago
*** Bug 161086 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

15 years ago
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.