Closed Bug 443693 Opened 12 years ago Closed 12 years ago

Integer overflow in info_callback() processing animated PNG


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






(Reporter: halb.halb, Assigned: vlad)


(Keywords: fixed1.9.0.2, verified1.9.1, Whiteboard: [sg:critical?])


(2 files)

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

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
I couldn't attach it because it was too large(600 kb).
Note: don't try to copy the integer overflow checks from gfxASurface::CheckSurfaceSize.  See for the reason and some alternatives.
Comment on attachment 328192 [details]
crashing testcase

preserving testcase in case server goes away
Attachment #328192 - Attachment is patch: false
Attachment #328192 - Attachment mime type: text/plain → image/png
Confirming bug, does not affect Firefox 2.
Assignee: nobody → vladimir
Component: General → ImageLib
Ever confirmed: true
Flags: wanted1.8.1.x-
Flags: blocking1.9.0.2?
Product: Firefox → Core
Whiteboard: [sg:critical?]
Version: unspecified → Trunk
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.
Summary: Integer overflows in info_callback() processing animated PNG → Integer overflow in info_callback() processing animated PNG
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.
(In reply to comment #1)
> I couldn't attach it because it was too large(600 kb). 

Since it's attached to this bugzilla entry now, I'll remove it from the site.
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
Assignee: vladimir → pavlov
Flags: blocking1.9.1?
Joe, can you take a look at this?  Should be a straightforward fix.
Yep, super simple, add overflow check before allocating.
Assignee: pavlov → vladimir
Attachment #328773 - Flags: review?(joe)
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
Comment on attachment 328773 [details] [diff] [review]
check for overflow

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

Attachment #328773 - Flags: review?(joe) → review+
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.
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Flags: blocking1.9.0.2? → blocking1.9.0.2+
Vlad, does this patch apply cleaning to 1.9.0? Can you please request approval if so?
Whiteboard: [sg:critical?] → [sg:critical?][need 1.8/1.9 patches]
Whiteboard: [sg:critical?][need 1.8/1.9 patches] → [sg:critical?][need 1.9.0 patches]
Comment on attachment 328773 [details] [diff] [review]
check for overflow

Doh, yes, this applies cleanly to 1.9.
Attachment #328773 - Flags: approval1.9.0.2?
Comment on attachment 328773 [details] [diff] [review]
check for overflow

Approved for Please land in CVS. a=ss
Attachment #328773 - Flags: approval1.9.0.2? → approval1.9.0.2+
Whiteboard: [sg:critical?][need 1.9.0 patches] → [sg:critical?]
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
Keywords: fixed1.9.0.2
Flags: wanted1.8.0.x-
Group: core-security
for future reference, this is CVE-2008-4064
(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 "" 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?
The image is invalid, so that's probably the desired behavior.
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
You need to log in before you can comment on or make changes to this bug.