Last Comment Bug 570451 - (CVE-2010-1205) segmentation fault when viewing a malformed PNG image
(CVE-2010-1205)
: segmentation fault when viewing a malformed PNG image
Status: RESOLVED FIXED
[sg:critical?][critsmash:patch]
: crash, fixed1.9.0.20, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: All All
: -- critical (vote)
: mozilla2.0b1
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
http://www.ee.oulu.fi/~aki/spark.png
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-06 23:40 PDT by Aki Helin
Modified: 2013-05-18 18:14 PDT (History)
28 users (show)
mats: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+
.7+
.7-fixed
.11+
.11-fixed


Attachments
The png in question (114 bytes, image/png)
2010-06-07 11:19 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
no flags Details
v00: check for out-of-bounds row_num in libpr0n (checked in) (962 bytes, patch)
2010-06-15 21:48 PDT, Glenn Randers-Pehrson
joe: review+
vladimir: superreview+
dveditz: approval1.9.2.7+
dveditz: approval1.9.1.11+
dveditz: approval1.9.0.next+
Details | Diff | Review
v01: check for out-of-bounds row_num in libpng (816 bytes, patch)
2010-06-16 06:17 PDT, Glenn Randers-Pehrson
joe: review-
Details | Diff | Review
v02: check for out-of-bounds row_num in libpng (956 bytes, patch)
2010-06-19 05:37 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review
v03: check for out-of-bounds row_num in libpng (10.22 KB, patch)
2010-06-20 04:36 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review
v04: check for out-of-bounds row_num in libpng (5.58 KB, patch)
2010-06-21 05:23 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review
v05: check for out-of-bounds row_num in libpng (5.58 KB, patch)
2010-06-25 11:58 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review

Description Aki Helin 2010-06-06 23:40:19 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/534.1 (KHTML, like Gecko) Chrome/6.0.427.0 Safari/534.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100606 Minefield/3.7a5pre

A segmentation fault occurs when the mentioned image is opened in x86 Ubuntu 10.04 using either the default version of Firefox or the current nightly build. The error occurs in gfxASurface::SurfaceDestroyFunc. 

The backtrace from the nightly build is:

Program received signal SIGSEGV, Segmentation fault.
0x00d2a99b in gfxASurface::SurfaceDestroyFunc(void*) () from ./libxul.so
(gdb) bt
#0  0x00d2a99b in gfxASurface::SurfaceDestroyFunc(void*) () from ./libxul.so
#1  0x010a86a8 in ?? () from ./libxul.so
#2  0x00d97b14 in ?? () from ./libxul.so
#3  0x00d2add3 in gfxASurface::Release() () from ./libxul.so
#4  0x00000000 in ?? ()

The kinda worrysome looking 0 return address does not show up in the backtrace for default Firefox.

Not tested in 64-bit builds or other platforms yet.


Reproducible: Always

Steps to Reproduce:
$ firefox http://www.ee.oulu.fi/~aki/spark.png
Actual Results:  
$ firefox http://www.ee.oulu.fi/~aki/spark.png
Segmentation fault
$ 


Expected Results:  
$ firefox http://www.ee.oulu.fi/~aki/spark.png
Comment 1 Brandon Sterne (:bsterne) 2010-06-07 08:30:31 PDT
Here's the backtrace using a DEBUG build.  Whatever's being handed to gfxASurface::SurfaceDestroyFunc is bogus and we're crashing when trying to delete it.

Program received signal SIGSEGV, Segmentation fault.
0x018b5105 in gfxASurface::SurfaceDestroyFunc (data=0xff00ffff)
    at /build/m-c/mozilla/gfx/thebes/src/gfxASurface.cpp:131
131	    delete surf;
(gdb) bt
#0  0x018b5105 in gfxASurface::SurfaceDestroyFunc (data=0xff00ffff)
    at /build/m-c/mozilla/gfx/thebes/src/gfxASurface.cpp:131
#1  0x0193b4a0 in _cairo_user_data_array_fini (array=0xb2290ddc)
    at /build/m-c/mozilla/gfx/cairo/cairo/src/cairo-array.c:389
#2  0x019607ea in *INT__moz_cairo_surface_destroy (surface=0xb2290dc0)
    at /build/m-c/mozilla/gfx/cairo/cairo/src/cairo-surface.c:583
#3  0x018b509e in gfxASurface::Release (this=0xabf67910)
    at /build/m-c/mozilla/gfx/thebes/src/gfxASurface.cpp:112
(gdb) p surf
$1 = (gfxASurface *) 0xff00ffff
Comment 2 Joe Drew (not getting mail) 2010-06-07 10:55:46 PDT
Glenn, can you take a look at this?
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-06-07 11:19:06 PDT
Created attachment 449662 [details]
The png in question
Comment 4 Aki Helin 2010-06-07 11:20:55 PDT
The image at http://www.ee.oulu.fi/~aki/spark-2.png causes a similar crash. Slightly different trace, but I'm guessing it's the same issue.
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-06-07 11:30:09 PDT
For what it's worth, my local debug builds I see crashes in various other places too (DOM code, imgFrame::EstimateMemoryUsed with |this| being jemalloc freed markers), plus assertions like so:

###!!! ASSERTION: Don't ask for a frame > 0 if we're not animated!: 'framenum == 0', file ../../../../mozilla/modules/libpr0n/src/imgContainer.cpp, line 373

When those assertions fire, framenum in imgContainer::GetImgFrame is 0xff00fffe, because numFrames is 0xff00ffff in nsPNGDecoder::EndImageFrame.

And that's because mImage.mRawPtr->mFrames.Length() is 0xff00ffff at that point (why, I have no idea).
Comment 6 Glenn Randers-Pehrson 2010-06-07 11:40:03 PDT
According to "pngcrush -n -v spark.png" it has "Extra compressed data".  This causes libpng-1.4 to issue two "benign errors".  Such errors can be handled as warnings or (by default) as errors.  Mozilla allows libpng to handle them by default.

@aki, are you using the embedded libpng or the system libpng on Ubuntu?
I'm using Ubuntu 10.04 with Firefox 3.6.3 as supplied by Ubuntu, and am not seeing a crash; I see a 2x2 green image.  Same with spark-2.png except it't 1x1.

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.3) Gecko/20100423 Ubuntu/10.04 (lucid) Firefox/3.6.3
Comment 7 Vladimir Vukicevic [:vlad] [:vladv] 2010-06-07 11:57:01 PDT
sounds like valgrind might be able to help here; cc'ing sewardj to see if he has a handy setup..
Comment 8 Glenn Randers-Pehrson 2010-06-07 12:06:46 PDT
A recent minefield build from source (2010 0528) crashes for me on the image, on Ubuntu 10.04.

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.3a5pre) Gecko/20100528 Minefield/3.7a5pre
Comment 9 Boris Zbarsky [:bz] (Out June 25-July 6) 2010-06-07 12:08:38 PDT
For what it's worth, I got crashes using in-tree libpng on both F12 and Mac.
Comment 10 Aki Helin 2010-06-07 12:19:01 PDT
@glenn, system libpng (/lib/libpng12.so.0).
Comment 11 Julian Seward [:jseward] 2010-06-07 13:06:38 PDT
Out of range write.  This is with a TM build of 31 May which I
happened to have hanging around.

Invalid write of size 4
   at 0x56A884D: row_callback(png_struct_def*, unsigned char*, unsigned int, int) (nsPNGDecoder.cpp:861)
   by 0x67BDA6E: MOZ_PNG_push_have_row (pngpread.c:1875)
   by 0x67BDE55: MOZ_PNG_push_proc_row (pngpread.c:1279)
   by 0x67BE3DA: MOZ_PNG_proc_IDAT_data (pngpread.c:1040)
   by 0x67BE7D5: MOZ_PNG_push_read_IDAT (pngpread.c:996)
   by 0x67BF28C: MOZ_PNG_proc_some_data (pngpread.c:70)
   by 0x67BF2E9: MOZ_PNG_process_data (pngpread.c:41)
   by 0x56A7FE0: nsPNGDecoder::Write(char const*, unsigned int) (nsPNGDecoder.cpp:432)
   by 0x56888C0: imgContainer::WriteToDecoder(char const*, unsigned int) (imgContainer.cpp:2240)
   by 0x5688C5E: imgContainer::AddSourceData(char const*, unsigned int) (imgContainer.cpp:1235)
   by 0x5685C05: imgContainer::WriteToContainer(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) (imgContainer.cpp:2682)
   by 0x66C7CD1: nsPipeInputStream::ReadSegments(unsigned int (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) (nsPipe3.cpp:799)
 Address 0x183fe2a0 is 0 bytes after a block of size 16 alloc'd
   at 0x4C237FF: calloc (vg_replace_malloc.c:467)
   by 0x7829206: moz_calloc (mozalloc.cpp:113)
   by 0x67696B8: gfxImageSurface::gfxImageSurface(gfxIntSize const&, gfxASurface::gfxImageFormat) (gfxImageSurface.cpp:101)
   by 0x568E7D7: imgFrame::Init(int, int, int, int, gfxASurface::gfxImageFormat, signed char) (imgFrame.cpp:241)
   by 0x5689A1D: imgContainer::InternalAddFrame(unsigned int, int, int, int, int, gfxASurface::gfxImageFormat, unsigned char, unsigned char**, unsigned int*, unsigned int**, unsigned int*) (imgContainer.cpp:722)
   by 0x5689E80: imgContainer::AppendFrame(int, int, int, int, gfxASurface::gfxImageFormat, unsigned char**, unsigned int*) (imgContainer.cpp:786)
   by 0x56A92A1: nsPNGDecoder::CreateFrame(unsigned int, unsigned int, int, int, gfxASurface::gfxImageFormat) (nsPNGDecoder.cpp:136)
   by 0x56A989B: info_callback(png_struct_def*, png_info_struct*) (nsPNGDecoder.cpp:732)
   by 0x67BDA33: MOZ_PNG_push_have_info (pngpread.c:1861)
   by 0x67BECC0: MOZ_PNG_push_read_chunk (pngpread.c:435)
   by 0x67BF2A4: MOZ_PNG_proc_some_data (pngpread.c:64)
   by 0x67BF2E9: MOZ_PNG_process_data (pngpread.c:41)
Comment 12 Daniel Veditz [:dveditz] 2010-06-08 16:57:30 PDT
Crashes 3.5.10pre, too.
Comment 13 Aki Helin 2010-06-09 01:58:16 PDT
A few more cases for testing a fix are at http://www.ee.oulu.fi/~aki/png-cases/. I'll add some more later this week when I'm back at OUSPG.
Comment 14 Joe Drew (not getting mail) 2010-06-11 14:50:38 PDT
Glenn, will you be able to look into this?
Comment 15 Joe Drew (not getting mail) 2010-06-15 14:14:20 PDT
I guess Glenn's busy - Jeff'll look into this.
Comment 16 Glenn Randers-Pehrson 2010-06-15 17:46:54 PDT
Good guess.  I'm here but busy with something else.
Comment 17 Glenn Randers-Pehrson 2010-06-15 17:52:26 PDT
Good guess.  I'm here but busy with something else.  I am suspecting something is wrong with the png_benign_error() handling, but comment #10 says the system libpng-1.2 is also acting up.  Note that if MOZ_PNG_stuff is in the traceback then it is the embedded libpng, even if you requested system libpng via configure (it might have fallen back on the embedded library to get the APNG support).  If both embedded and system libpng exhibit the bug, then look for a problem in libpr0n/decoders/png.
Comment 18 Jeff Muizelaar [:jrmuizel] 2010-06-15 18:23:51 PDT
Looks like our row_callback is being called with row_num == mFrameRect.height (2) which causes us to write beyond the image data buffer. I'm not yet sure why libpng calls us with row_num == 2.
Comment 19 Glenn Randers-Pehrson 2010-06-15 20:01:20 PDT
Confirmed that the bug is within libpng.  The rpng2-x demo program (progressive reading) provided with libpng in contrib/gregbook segfaults when trying to read spark.png.
Comment 20 Glenn Randers-Pehrson 2010-06-15 20:27:23 PDT
Libpng's example.c contains this check at the
beginning of the row_callback:

/* Check if row_num is in bounds. */
   if ((row_num >= 0) && (row_num < height))
   {

That check is omitted from libpng's contrib/gregbook/readpng2.c
and also from libpr0n's decoders/png/nsPNGDecoder.cpp, which
was modeled after the gregbook code.
Comment 21 Glenn Randers-Pehrson 2010-06-15 20:38:32 PDT
rpng2-x does *not* crash after I added the check if row_num is in bounds.

I'll put together a patch to nsPNGDecoder.cpp

I don't have access to the TryServer anymore (rules seem to have changed)
so someone else will have to run that.
Comment 22 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-06-15 20:48:27 PDT
(In reply to comment #21)
> I don't have access to the TryServer anymore (rules seem to have changed)
> so someone else will have to run that.

The only rule changes we've made recently made the rules less restrictive, so if you had access before and lost it, you should file a bug to sort that out:

https://bugzilla.mozilla.org/enter_bug.cgi?product=mozilla.org&component=Server%20Operations
Comment 23 Jeff Muizelaar [:jrmuizel] 2010-06-15 21:00:20 PDT
Shouldn't the caller of row_callback ensure that row_num is in bounds? row_num >= height seems to be handle pretty inconsistently by users of libpng.

These users don't seem to check:
 dillo-0.8.6/src/png.c
 WebCore/platform/image-decoders/png/PNGImageDecoder.cpp
 links-2.1pre14/png.c

These users check but assume or assert that the condition won't happen:
 trunk/gears/base/common/png_utils.cc
 chromium/gfx/codec/png_codec.cc 

These users do the check:
 gdk-pixbuf/io-png.c
Comment 24 Glenn Randers-Pehrson 2010-06-15 21:12:19 PDT
I don't know why the check is in libpng/example.c but someone must
have experienced a problem or realized that a problem was possible.

A lot of applications are probably built with the gregbook code,
as is libpr0n/decoders/nsPNGDecoder.cpp, and would be vulnerable

The test was added to libpng/example.c in libpng-1.2.1beta01, in
October 2001.
Comment 25 Glenn Randers-Pehrson 2010-06-15 21:30:49 PDT
The revised code for example.c, with the check for row_num out of
bounds, was contributed by "Capt. Taura M." on 10 September 2001.
It was meant to clarify the example; there is no mention of anything
crashing, but there was a problem with reading interlaced PNGs
in The GIMP which may have been related.
Comment 26 Glenn Randers-Pehrson 2010-06-15 21:48:47 PDT
Created attachment 451471 [details] [diff] [review]
v00: check for out-of-bounds row_num in libpr0n (checked in)
Comment 27 Aki Helin 2010-06-16 05:14:52 PDT
I originally also opened a Chromium issue for this case, and can tell there about your findings. What about other the affected programs? Will libpng later check row_num before calling the callback, or should the users check the validity? If the latter is the way to go, I could give the files to some friends at CERT-FI for checking if other projects or vendors are affected.
Comment 28 Glenn Randers-Pehrson 2010-06-16 05:30:47 PDT
I have notified the libpng developers.  As a minimum we will change the example code in contrib/gregbook/readpng2.c upon which a lot of applications seem to be based.  I assume that we will fix libpng to check internally as well, if we can.

It looks as though we simply could insert a check for row_num >= png_ptr->height
at the entry to png_push_have_row() in pngpread.c
Comment 29 Glenn Randers-Pehrson 2010-06-16 05:53:05 PDT
Interestingly, my Firefox 3.6.3 on Ubuntu doesn't crash when I look at
aki's page full of bad PNGs.  However, later on when I try to close the
tab, Firefox is blown away.  Reopening Firefox gives the "well, this is
embarassing" window.  "Restore" seems to work.
Comment 30 Glenn Randers-Pehrson 2010-06-16 05:54:24 PDT
Someone, please test the v00 patch.  I've lost access to the Tryserver.
Comment 31 Glenn Randers-Pehrson 2010-06-16 06:11:11 PDT
I experienced the "well this is embarassing" on Windows XP, Firefox 3.7a3pre, when I powered up my computer and reopened Firefox this morning after viewing one of aki's images yesterday and shutting down the computer yesterday.  So I'm changing it from X86/Linux to All/All.
Comment 32 Glenn Randers-Pehrson 2010-06-16 06:17:15 PDT
Created attachment 451560 [details] [diff] [review]
v01: check for out-of-bounds row_num in libpng

Patch for the embedded libpng
Comment 33 Glenn Randers-Pehrson 2010-06-16 06:21:18 PDT
Please test the v01 patch as well.  v01 fixes the embedded libpng while v00 (still needed) makes libpr0n catch the problem when the system libpng is used.  We should probably apply both.
Comment 34 Jeff Muizelaar [:jrmuizel] 2010-06-16 06:27:52 PDT
I've pushed v00 to try. I'm not thrilled about pushing to try though because it can potentially reveal the problem, so I'm holding off pushing v01 and the combination. I think we can probably land v00 everywhere and then deal with v01 after we've done a release.
Comment 35 Glenn Randers-Pehrson 2010-06-16 06:52:04 PDT
That's OK; v00 takes care of both embedded and system.  The combo will be redundant when using the embedded libpng, so we might want to #ifdef out
the test in libpr0n when using the embedded libpng.  But as you say, that can
wait.
Comment 36 Joe Drew (not getting mail) 2010-06-16 11:04:17 PDT
Comment on attachment 451560 [details] [diff] [review]
v01: check for out-of-bounds row_num in libpng

OK, but why did row_number grow bigger than the height?
Comment 37 Jeff Muizelaar [:jrmuizel] 2010-06-16 11:07:19 PDT
Seems to pass the try server without issue.
Comment 38 Glenn Randers-Pehrson 2010-06-16 11:15:24 PDT
(In reply to comment #36)
> (From update of attachment 451560 [details] [diff] [review])
> OK, but why did row_number grow bigger than the height?

Don't know.  The libpng group is checking into it.  The
v00 and v01 patches fix the symptoms but don't cure the
underlying problem.  Hopefully we will come up with an
explanation and a real fix.  If anyone here has an idea,
feel free to post it here, send it to me, or better yet
(after this bug's security flag is cleared!),
join the png-mng-implement mailing list at sourceforge
and post it there.
Comment 39 Daniel Veditz [:dveditz] 2010-06-17 09:40:48 PDT
Let's try to get this into the branch builds ASAP since the problem is fairly obvious from the patch. Please hold off on the trunk landing for a bit and we'll try to land everywhere next week. 

Or will our landing "out" the problem for every libpng using-system everywhere, not just Mozilla clients? If so we need to coordinate w/Glenn
Comment 40 Glenn Randers-Pehrson 2010-06-17 14:51:21 PDT
It will reveal the problem that exists in every libpng-using system that does progressive reading (i.e., using callbacks) and that follows the libpng/gregbook/readpng2.c example and not the libpng/example.c.  I guess CERT should be notified right away.  CERT will probably want 3 weeks before going public with the bug.
Comment 41 christian 2010-06-18 10:15:16 PDT
Comment on attachment 451471 [details] [diff] [review]
v00: check for out-of-bounds row_num in libpr0n (checked in)

a=LegNeato for 1.9.2.6 and 1.9.1.11. Please land on mozilla-1.9.2 default and mozilla-1.9.1 default.
Comment 42 christian 2010-06-18 10:17:31 PDT
Comment on attachment 451471 [details] [diff] [review]
v00: check for out-of-bounds row_num in libpr0n (checked in)

Actually, setting back approvals to "?". We don't want this landed until we check with the libpng folks and understand when they are shipping their own fix.
Comment 43 Daniel Veditz [:dveditz] 2010-06-18 10:37:55 PDT
but code-freeze for 1.9.2.6 is likely in a week, so would be nice to know by then.
Comment 44 Aki Helin 2010-06-18 11:59:16 PDT
Chromium also has a patch waiting. I'll post there once you decide to apply the row check patch and/or know when a libpng update is coming.
Comment 45 Joe Drew (not getting mail) 2010-06-18 21:32:42 PDT
Comment on attachment 451560 [details] [diff] [review]
v01: check for out-of-bounds row_num in libpng

We don't need to take this, since we have v00. Let's just wait for a proper fix from the libpng group.
Comment 46 Glenn Randers-Pehrson 2010-06-19 05:37:18 PDT
Created attachment 452457 [details] [diff] [review]
v02: check for out-of-bounds row_num in libpng

v02 is the patch currently being considered by the libpng group.  With it, the v00 patch isn't needed, except when using an unpatched system libpng.
Comment 47 Aki Helin 2010-06-20 00:04:15 PDT
The Chromium team would really like to apply the equivalent of v00 asap with some non-suspicious message until the official libpng fix is out. Any chance syncing this with Mozilla?
Comment 48 Glenn Randers-Pehrson 2010-06-20 04:36:42 PDT
Created attachment 452548 [details] [diff] [review]
v03: check for out-of-bounds row_num in libpng

v03 is a more complete patch that the libpng group is now considering.
Comment 49 Reed Loden [:reed] (use needinfo?) 2010-06-20 06:44:29 PDT
dveditz, can you go ahead and assign a CVE for this issue?
Comment 50 Aki Helin 2010-06-20 06:48:39 PDT
If you want to try a patched version against more similar data before
releasing: 

mkdir -p $HOME/pngtest/{samples,fuzzed}
cd $HOME/pngtest
curl http://www.schaik.com/pngsuite/PngSuite.tar.gz | tar -C samples -zxvf -
curl http://ouspg.googlecode.com/files/radamsa-0.1.c.gz | gzip -d | gcc -O2 -o
radamsa -x c -
./radamsa -e surfy -n 1000 -o fuzzed/%f-%i.png samples/*.png

and then test the files coming to fuzzed/*.png, and remove when done to get
more. For example $ for file in fuzzed/*.png; do echo "<img src=$file>"; done >
all.html; firefox all.html. Leaving out the "-e surfy" gives more varied
changes, but that module is the one usually triggering this issue.
Comment 51 Glenn Randers-Pehrson 2010-06-21 05:23:24 PDT
Created attachment 452699 [details] [diff] [review]
v04: check for out-of-bounds row_num in libpng

I inadvertently removed some of the APNG support via the v03 patch.
Comment 52 Glenn Randers-Pehrson 2010-06-21 08:01:18 PDT
I've submitted a report to CERT:


An extra row of data in the IDAT chunk of a
png file can cause libpng applications that
use the progressive reader (with a row callback
function) to crash, if the row callback function
does not check for an out-of-bounds row number.

A workaround is to check for row_num greater
than or equal to the image height, at the
beginning of the user-supplied row callback
function.

All versions of libpng are vulnerable.

Libpng versions 1.2.44 and 1.4.3 will be
released shortly.  Libpng-1.0.x is no longer
supported and will not be updated.

A patch for Mozilla/firefox is being tested.

Glenn
Comment 53 Reed Loden [:reed] (use needinfo?) 2010-06-21 08:10:16 PDT
(In reply to comment #52)
> I've submitted a report to CERT:

Is CERT going to assign a CVE, or should we assign one out of our pool?
Comment 54 Glenn Randers-Pehrson 2010-06-21 08:16:56 PDT
I only wrote to CERT 15 minutes ago.  If dveditz wants to assign one,
go ahead.  Post the number here and I'll follow up to CERT.
My message to CERT was entitled GRP-2010-0001
Comment 55 Daniel Veditz [:dveditz] 2010-06-21 09:35:41 PDT
call it CVE-2010-1205.
Comment 56 Daniel Veditz [:dveditz] 2010-06-21 10:11:26 PDT
(In reply to comment #46)
> v00 patch isn't needed, except when using an unpatched system libpng.

We should take it anyway because people do build with system libpng.
Comment 57 Daniel Veditz [:dveditz] 2010-06-21 10:26:16 PDT
Comment on attachment 452699 [details] [diff] [review]
v04: check for out-of-bounds row_num in libpng

is this ready for a review? Or does it not need one because it's an official upstream patch?
Comment 58 Glenn Randers-Pehrson 2010-06-21 10:47:23 PDT
It's not yet an official upstream patch.  I'm hoping to see some test
results both here and on png-mng-implement at lists.sourceforge.net
but so far have only seen results from myself and one other person.
#:-(   ---  but the results so far are sucessful #:-)

Let me know exactly when you plan to land a patched Firefox release;
we will no doubt be able to coordinate the release of libpng-1.4.3
at the same time, along with 1.2.44 (legacy) and 1.5.0beta29 (devel).
Comment 59 Glenn Randers-Pehrson 2010-06-21 11:00:52 PDT
I know about the typos in the comments but those won't affect testing.
Comment 60 Daniel Veditz [:dveditz] 2010-06-23 10:38:18 PDT
Neither Chrome 5 nor Safari 5 on mac crashed for me on these testcases.
IE 8 on Windows did not crash
Chrome 5 on windows did crash, but Aki indicated they're ready to go with a patch too.

I think we need to land v00 everywhere now (so it makes 1.9.2.6/1.9.1.11) and upgrade libpng later.
Comment 61 Daniel Veditz [:dveditz] 2010-06-23 10:39:29 PDT
Comment on attachment 451471 [details] [diff] [review]
v00: check for out-of-bounds row_num in libpr0n (checked in)

Approved for 1.9.2.6 and 1.9.1.11, a=dveditz for release-drivers

Please land with an innocuous comment, that if possible keeps the attention on our own code and away from libpng.
Comment 62 Glenn Randers-Pehrson 2010-06-23 11:16:22 PDT
I think libpng-1.4.3 is ready to go with v04 (with typos fixed) whenever you land v00.  libpng-1.4.3 will also fix a memory leak with the sCAL chunk, that does not affect you since libimg never processes sCAL.
Comment 63 Aki Helin 2010-06-23 11:19:34 PDT
I just posted to Chromium that you will apply the workaround and libpng is ready for release.
Comment 64 Glenn Randers-Pehrson 2010-06-23 11:24:05 PDT
By the way, I have not gotten a reply from CERT yet (I have learned not to expect a timely reply from them).  Does anyone here know if they are aware of this bug?
Comment 65 Aki Helin 2010-06-23 11:47:06 PDT
I gave the CVE number and a rough description informally to our contact at CERT-FI, so at least they do. Let me know if they should help with an advisory etc.
Comment 66 Reed Loden [:reed] (use needinfo?) 2010-06-23 11:55:19 PDT
Mozilla also has contacts with vendor-sec if you want to go that route.
Comment 67 Glenn Randers-Pehrson 2010-06-25 11:58:05 PDT
Created attachment 454102 [details] [diff] [review]
v05: check for out-of-bounds row_num in libpng

Avoids infinite loop while decoding a certain PNG file.
This is the libpng-1.4.3rc03 version.
Comment 68 :Ehsan Akhgari (busy, don't ask for review please) 2010-06-25 12:25:33 PDT
http://hg.mozilla.org/mozilla-central/rev/377f46f4d000
Comment 70 :Ehsan Akhgari (busy, don't ask for review please) 2010-06-25 13:32:04 PDT
Bustage fix landed on 1.9.1: <http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e969f299ac19>
Comment 71 Glenn Randers-Pehrson 2010-06-26 04:41:18 PDT
This bug has been fixed in libpng-1.4.3 which was released yesterday.
<http://libpng.sourceforge.net>
Comment 72 Glenn Randers-Pehrson 2010-06-26 05:23:48 PDT
See also bug #564792
Comment 73 Glenn Randers-Pehrson 2010-06-30 05:41:59 PDT
verified fixed on Ubuntu-10.4 which just received a push of Firefox-3.6.6

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.6) Gecko/20100628 Ubuntu/10.04 (lucid) Firefox/3.6.6
Comment 74 Glenn Randers-Pehrson 2010-06-30 05:44:56 PDT
Comment on attachment 454102 [details] [diff] [review]
v05: check for out-of-bounds row_num in libpng

Marking the v05 patch as obsolete (by the release of libpng-1.4.3)
Comment 75 Reed Loden [:reed] (use needinfo?) 2010-06-30 11:07:21 PDT
(In reply to comment #73)
> verified fixed on Ubuntu-10.4 which just received a push of Firefox-3.6.6
> 
> Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.6) Gecko/20100628
> Ubuntu/10.04 (lucid) Firefox/3.6.6

Uh, what? Firefox 3.6.6 doesn't include the patch for this. Unless they're using system libpng and have already patched it with the newer release, you shouldn't see this as fixed.
Comment 77 Glenn Randers-Pehrson 2010-06-30 11:46:19 PDT
I guess it's possible that I'm using a patched libpng.  All I know is that I received Firefox 3.6.6 from Ubuntu, and didn't see a crash.  I'll check further when I get home this evening.
Comment 78 Glenn Randers-Pehrson 2010-06-30 12:51:12 PDT
-rw-r--r-- 1 root root 158736 2010-03-19 15:24
ldd says my firefox-3.6.6 is using libpng12.so.0.
That is linked to libpng.1.2.42, not patched.

So I don't know why I don't see the crash now.
Comment 79 Aki Helin 2010-06-30 13:03:29 PDT
My just now patched Ubuntu is using libpng 1.2.42-1ubuntu2. The bundled Firefox 3.6.6 crashes on spark.png.
Comment 80 Daniel Veditz [:dveditz] 2010-06-30 13:12:40 PDT
Glenn: your system is magic. We have not released a fix for this yet. What may not be obvious from comments alone is that "fixed in 1.9.2.6" became "fixed in 1.9.2.7" when the inserted plugin-hang-fix release took the 1.9.2.6-based name (but the "status1.9.2" field was updated to reflect the correct state).
Comment 81 Aki Helin 2010-06-30 13:40:17 PDT
Correction, x86 crashes on load, whereas on x86_64 Firefox seems to work after opening the images, but crashes if closed or the page is loaded a few times, apparently whenever something accesses whatever was corrupted next to the broken image.
Comment 82 Glenn Randers-Pehrson 2010-06-30 16:55:31 PDT
OK, I opened spark.png ten times with no problem but on the 11th, in a new tab,  it crashed.

Magic, indeed.    #:-(

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.6) Gecko/20100628 Ubuntu/10.04 (lucid) Firefox/3.6.6
Comment 83 Glenn Randers-Pehrson 2010-07-01 05:48:44 PDT
(In reply to comment #82)
> OK, I opened spark.png ten times

I should not have done that.  The end result was that I've completely lost access to the ubuntu machine, except via a "generic-recovery" reboot to a CLI, or via CTRL-ALT-F1.  I can't get it to give me a login prompt (just a purple screen-of-death with highlights and no login dialog box).  Possibly related is that the crash happened while the update manager was trying to update sudo and some other kernel stuff, but I'm suspecting spark.png.
Comment 84 Glenn Randers-Pehrson 2010-07-05 09:01:15 PDT
A pointer to spark.png has been posted to the chromium bug tracker and to the png-mng-implement mailing list.  So I suppose the security flag can be cleared here.
Comment 85 Aki Helin 2010-07-05 12:00:09 PDT
In case it is of any help, I removed the images to which they linked a while ago. I think only this thread has a copy of the original file.
Comment 86 Glenn Randers-Pehrson 2010-07-07 10:20:31 PDT
(In reply to comment #85)
> In case it is of any help, I removed the images to which they linked a while
> ago. I think only this thread has a copy of the original file.

I have a copy but it is on my old Ubuntu disk. I'm back in business now;
put in a new disc and did a fresh install of Ubuntu Lucid on it.  I can mount the old disc and read the files on it, but never succeeded in getting the "half-configured" gdm and gedit running again, evidently due to the nasty crash caused by this bug.

Note that I opened a separate bug #564792 about upgrading to libpng-1.4.3 on the trunk.  For other branches, applying the v05 patch from here should be sufficient.
Comment 87 [On PTO until 6/29] 2010-07-16 13:41:43 PDT
Verified for 1.9.2 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7 using the attached image. It crashes 1.9.2.6 cleanly.

Verified crashing in 1.9.1.10 and fixed in 1.9.1.11 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.11) Gecko/20100701 Firefox/3.5.11 as well.
Comment 88 Daniel Veditz [:dveditz] 2010-07-22 19:14:52 PDT
Comment on attachment 451471 [details] [diff] [review]
v00: check for out-of-bounds row_num in libpr0n (checked in)

Approved for 1.9.0.20, a=dveditz
Comment 89 Smokey Ardisson (offline for a while; not following bugs - do not email) 2010-07-24 15:34:23 PDT
Checking in modules/libpr0n/decoders/png/nsPNGDecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/png/nsPNGDecoder.cpp,v  <--  nsPNGDecoder.cpp
new revision: 1.86; previous revision: 1.85
done
Comment 90 Mats Palmgren (:mats) 2013-05-18 09:43:59 PDT
Crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ea5c47f41ad
Comment 91 Phil Ringnalda (:philor) 2013-05-18 18:14:58 PDT
https://hg.mozilla.org/mozilla-central/rev/9ea5c47f41ad

Note You need to log in before you can comment on or make changes to this bug.