443693 - Integer overflow in info_callback() processing animated PNG

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P1)

Description

[Drew Yao]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_4; en-us) AppleWebKit/525.18 (KHTML, like Gecko) Version/3.1.1 Safari/525.20
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0


Integer overflows in info_callback() processing animated PNG

There are 2 integer overflows leading to heap buffer overflows in info_callback() in nsPNGDecoder.cpp.  I believe it's only exploitable when parsing APNGs, so I think this only affects FF3.  I have not looked at the code for FF 2.  I'm guessing this might affect Thunderbird, but I haven't checked.  Please credit Drew Yao of Apple Product Security.

in info_callback():
  if (png_get_first_frame_is_hidden(png_ptr, info_ptr)) {
    decoder->mFrameIsHidden = PR_TRUE;
  } else {
    decoder->CreateFrame(0, 0, width, height, decoder->format);
  }
  
  if (decoder->mTransform &&
      (channels <= 2 || interlace_type == PNG_INTERLACE_ADAM7)) {
    PRUint32 bpp[] = { 0, 3, 4, 3, 4 };
    decoder->mCMSLine =
      (PRUint8 *)nsMemory::Alloc(bpp[channels] * width);
    if (!decoder->mCMSLine)
      longjmp(decoder->mPNG->jmpbuf, 5); // NS_ERROR_OUT_OF_MEMORY
  }

  if (interlace_type == PNG_INTERLACE_ADAM7) {
    decoder->interlacebuf = (PRUint8 *)nsMemory::Alloc(channels * width * height);

Both of the Allocs can overflow to an unexpectedly small number, leading to a buffer overflow later.  The reason I think this can only be triggered by a APNG is that if png_get_first_frame_is_hidden() is false, decoder->CreateFrame will get called.  That leads to gfxASurface::CheckSurfaceSize getting called, which checks if height * width would overflow.  To avoid this check the attacker needs to use an APNG with the first fcTL chunk missing, which causes png_get_first_frame_is_hidden() to be true.

When you run the attached test case png file, Firefox will crash.  Sometimes it might be a null deref.  I can demonstrate that the integer overflow occurs, leading to a heap buffer overflow, using libgmalloc(3) on Mac OS X.  libgmalloc places a guard page immediately after each heap allocation, causing a crash when you write past the end of it.



(gdb) set env DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib

(gdb) b nsPNGDecoder.cpp:648 if png_ptr->apng_flags == 1

(gdb) run


Breakpoint 1, info_callback (png_ptr=0x56ddde00, info_ptr=0x56de7f60) at /Volumes/data_apps/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp:648
648	  if (decoder->mTransform &&
(gdb) n
Current language:  auto; currently c++
657	  if (interlace_type == PNG_INTERLACE_ADAM7) {
(gdb) n
658	    decoder->interlacebuf = (PRUint8 *)nsMemory::Alloc(channels * width * height);
(gdb) p channels
$1 = 4
(gdb) p/x width
$2 = 0x8000
(gdb) p/x height
$3 = 0x8000
(gdb) p/x channels*width*height
$4 = 0x0
#overflow causes a 0-size buffer to be allocated
(gdb) n


(gdb) p decoder->interlacebuf
$11 = (PRUint8 *) 0x5700bff0 ""
(gdb) c
Continuing.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x5700c000
0x13480cd4 in MOZ_PNG_combine_row (png_ptr=0x56ddde00, row=0x5700bff0 "", mask=255) at /Volumes/data_apps/mozilla/modules/libimg/png/pngrutil.c:2472
2472	      png_memcpy(row, png_ptr->row_buf + 1,
(gdb) p row
$12 = (png_bytep) 0x5700bff0 ""
#The destination pointer for this memcpy, is the 0-size buffer which was allocated earlier.
(gdb) bt
#0  0x13463cd4 in MOZ_PNG_combine_row (png_ptr=0x5305de00, row=0x5328dff0 "", mask=255) at /Volumes/data_apps/mozilla/modules/libimg/png/pngrutil.c:2472
#1  0x13460f5d in MOZ_PNG_progressive_combine_row (png_ptr=0x5305de00, old_row=0x5328dff0 "", new_row=0x5306ffd1 "") at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:1746
#2  0x134361fc in row_callback (png_ptr=0x5305de00, new_row=0x5306ffd1 "", row_num=0, pass=0) at /Volumes/data_apps/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp:715
#3  0x13460f16 in MOZ_PNG_push_have_row (png_ptr=0x5305de00, row=0x5306ffd1 "") at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:1732
#4  0x1346066a in MOZ_PNG_push_proc_row (png_ptr=0x5305de00) at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:1029
#5  0x13460429 in MOZ_PNG_proc_IDAT_data (png_ptr=0x5305de00, buffer=0xf173e595 "x??y?\034?}??y??UgW??@\003??M\020 \001R?(??(?ƒ|??̄|??^Gػ????????\0373??#bg&<\023\033?3?Þ?ږ?;?aY?i?\"EJ?O", buffer_length=2655) at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:980
#6  0x13460239 in MOZ_PNG_push_read_IDAT (png_ptr=0x5305de00) at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:911
#7  0x1345e9e6 in MOZ_PNG_proc_some_data (png_ptr=0x5305de00, info_ptr=0x53067f60) at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:61
#8  0x1345e95d in MOZ_PNG_process_data (png_ptr=0x5305de00, info_ptr=0x53067f60, buffer=0xf173dff4 ",???.???C\f???????\031?\017h?쩉?X?\"?\032????u>????W?j?\v\023Zhͳ? L&?d?Q^\023??\005\nr?g\032?\032?6?\024?z\024?\017C? \004}38\022(?˔\020?e?.AF??\0340\006??\b?=Sc", buffer_size=4096) at /Volumes/data_apps/mozilla/modules/libimg/png/pngpread.c:36
#9  0x1343569b in ReadDataOut (in=0x46ea9f6c, closure=0x5305bfc0, fromRawSegment=0xf173dff4 ",???.???C\f???????\031?\017h?쩉?X?\"?\032????u>????W?j?\v\023Zhͳ? L&?d?Q^\023??\005\nr?g\032?\032?6?\024?z\024?\017C? \004}38\022(?˔\020?e?.AF??\0340\006??\b?=Sc", toOffset=20480, count=4096, writeCount=0xbfffd2c4) at /Volumes/data_apps/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp:335
#10 0x00370c5b in nsPipeInputStream::ReadSegments (this=0x46ea9f6c, writer=0x134355ee <ReadDataOut>, closure=0x5305bfc0, count=45056, readCount=0xbfffd564) at /Volumes/data_apps/mozilla/xpcom/io/nsPipe3.cpp:799



Reproducible: Always

Comment 1

[Drew Yao]

I couldn't attach it because it was too large(600 kb). 

http://idisk.mac.com/drew_yao-Public/firefox/png_overflow_case.png

Comment 2

[Drew Yao]

Note: don't try to copy the integer overflow checks from gfxASurface::CheckSurfaceSize.  See https://bugzilla.mozilla.org/show_bug.cgi?id=441372 for the reason and some alternatives.

Comment 3

[Daniel Veditz [:dveditz]]

Attachment: crashing testcase

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 328192 [details]
crashing testcase

preserving testcase in case server goes away

Comment 5

[Daniel Veditz [:dveditz]]

Confirming bug, does not affect Firefox 2.

Comment 6

[Daniel Veditz [:dveditz]]

br-874eb67a-4a66-11dd-b9d6-001cc45a2c28

Comment 7

[Daniel Veditz [:dveditz]]

crash stack eventually at bp-874eb67a-4a66-11dd-b9d6-001cc45a2c28

Comment 8

[Drew Yao]

I just realized the multiplication at the first alloc can't overflow, since width is constrained to be at most a million.  So there's only one integer overflow.

Comment 9

[Drew Yao]

I'd suggest something like this:
if (height > INT_MAX / (width * channels)) error

My understanding from looking at libpng is that width * channels could never overflow, and channels could never be negative, so those don't need to be checked.  It would be good to change channels to unsigned just out of principle.

Comment 10

[Drew Yao]

(In reply to comment #1)
> I couldn't attach it because it was too large(600 kb). 
> 
> http://idisk.mac.com/drew_yao-Public/firefox/png_overflow_case.png
> 

Since it's attached to this bugzilla entry now, I'll remove it from the site.

Comment 11

[Daniel Veditz [:dveditz]]

Vlad says => Stewart

We've had some breakpad server trouble recently, I doubt the one in comment 7 will ever be processed. Try bp-492e7f42-4ad9-11dd-8c6e-001cc4e2bf68

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Joe, can you take a look at this?  Should be a straightforward fix.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: check for overflow

Yep, super simple, add overflow check before allocating.

Comment 14

[Joe Drew (not getting mail)]

Comment on attachment 328773 [details] [diff] [review]
check for overflow

plz to be adding an example of the testcase to the regression tests!

r=JOEDREW!

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in to trunk; setting in-testsuite? since the crashing png needs to be added to crashtests, but only after this is fixed in 1.9.0.x and this bug is made public.

Comment 16

[Samuel Sidler (old account; do not CC)]

Vlad, does this patch apply cleaning to 1.9.0? Can you please request approval if so?

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 328773 [details] [diff] [review]
check for overflow

Doh, yes, this applies cleanly to 1.9.

Comment 18

[Samuel Sidler (old account; do not CC)]

Comment on attachment 328773 [details] [diff] [review]
check for overflow

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checking in modules/libpr0n/decoders/png/nsPNGDecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp,v  <--  nsPNGDecoder.cpp
new revision: 1.82; previous revision: 1.81
done

Comment 20

[Marc Bejarano]

for future reference, this is CVE-2008-4064

Comment 21

[Tony Chung [:tchung]]

(In reply to comment #4)
> (From update of attachment 328192 [details])
> preserving testcase in case server goes away

What happened to the attached image?   Clicking it says "The image "https://bugzilla.mozilla.org/attachment.cgi?id=328192" cannot be displayed, because it contains errors."

Do we have another testcase i can use to verify this fix on 1.9.1 and trunk?

Comment 22

[Martijn Wargers (dead)]

The image is invalid, so that's probably the desired behavior.

Comment 23

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090217 Shiretoko/3.1b3pre Ubiquity/0.1.5
and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090217 Minefield/3.2a1pre