Last Comment Bug 251381 - new libpng buffer overflow vulnerabilities
: new libpng buffer overflow vulnerabilities
Status: VERIFIED FIXED
[sg:fix]blocking1.4.3+
: fixed-aviary1.0, fixed1.4.3, fixed1.7.2, fixed1.7.5
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: Trunk
: All All
: -- major (vote)
: ---
Assigned To: Glenn Randers-Pehrson
:
Mentors:
http://scary.beasts.org/security/CESA...
: 257492 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-07-14 08:10 PDT by Glenn Randers-Pehrson
Modified: 2006-03-12 17:45 PST (History)
16 users (show)
bugs: blocking1.7.5+
bugs: blocking‑aviary1.0PR+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch for libpng tRNS chunk overflow (880 bytes, patch)
2004-07-22 21:13 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review
a more robust fix (765 bytes, patch)
2004-07-26 15:06 PDT, Glenn Randers-Pehrson
tor: review+
tor: superreview+
Details | Diff | Review
a more robust fix, with spaces after the commas (785 bytes, patch)
2004-07-27 15:16 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review
limit image dimensions, includes tRNS fix (1.75 KB, patch)
2004-08-02 09:42 PDT, tor
glennrp+bmo: review+
caillon: approval1.4.3+
Details | Diff | Review
patch to skip unused PNG chunks. (2.28 KB, patch)
2004-08-02 16:34 PDT, Glenn Randers-Pehrson
no flags Details | Diff | Review
patch to skip unused PNG chunks (but not sRGB!). (2.23 KB, patch)
2004-08-02 16:52 PDT, Glenn Randers-Pehrson
tor: review+
Details | Diff | Review
patch to skip unused PNG chunks (2) (2.14 KB, patch)
2004-08-03 10:39 PDT, Glenn Randers-Pehrson
tor: review+
caillon: approval1.4.3+
Details | Diff | Review
combined set of fixes (3.55 KB, patch)
2004-08-03 14:09 PDT, tor
mozilla: approval‑aviary+
mozilla: approval1.7.5+
Details | Diff | Review
bad png image for test case (8.37 KB, image/png)
2004-08-12 13:16 PDT, David Epstein
no flags Details

Description Glenn Randers-Pehrson 2004-07-14 08:10:31 PDT
I have been notified that there are some newly discovered buffer overflow
vulnerabilities in libpng.

http://scary.beasts.org/security/CESA-2004-001.txt
contains the bug report and patch.

The PLTE overflow can be used against mozilla.  The hIST, iCCP, sBIT, and
sPLT vulnerabilities would not affect mozilla because they are disabled
via the #defines in mozlibpngconf.h.

libpng-1.2.6 will be released shortly with patches but libpng-1.2.5 and
earlier, if used from the system, would be vulnerable.  Therefore we should
change the MOZPNG configuration variable to require libpng-1.2.6 as a minimum.

Glenn
Comment 1 Mike Shaver (:shaver -- probably not reading bugmail closely) 2004-07-14 08:24:51 PDT
Should we apply the patch from your report to our in-tree copy of libpng?
Comment 2 Glenn Randers-Pehrson 2004-07-14 08:46:52 PDT
Mike: Yes, I think so.   Definitely, the in-tree libpng has to be patched.
I have just tried on my FreeBSD platform and the patch was useable as is.
It contains an arbitrary reduction of the maximum image size (from 2^31-1
rows and columns to 2^24-1 rows and columns) but realistically no one will
be trying to display such large images.
Comment 3 Glenn Randers-Pehrson 2004-07-14 10:54:48 PDT
On FreeBSD5.0/gcc-3.2.1, using mozilla-1.8a to view the pngtest_bad.png
from the bug report, I get a segv core dump.

After applying the patch, I only get the text about
"cannot display the image because it contains errors" which is the
expected proper behavior.

I also tried to look at the image with Firefox 0.9.2 on Win XP Home
but it seemed to hang without showing anything.  After I killed
it and tried to run Firefox again, my profile was busy.

So I conclude that the reported vulnerability is real and the proposed
patch fixes it.
Comment 4 Glenn Randers-Pehrson 2004-07-14 11:00:44 PDT
Taking bug.

The original report that I received said that the bug is to be held
private until 4 August.  I will be releasing libpng-1.2.6 accordingly.
Comment 5 Daniel Veditz [:dveditz] 2004-07-14 11:33:26 PDT
assigning to Glenn per comment 4
Comment 6 chris hofmann 2004-07-20 11:04:40 PDT
what should the release plans be for this? should get patched on trunk, 1.7
branch and aviary 1.0 branch.
Comment 7 Christopher Aillon (sabbatical, not receiving bugmail) 2004-07-20 14:09:33 PDT
and 1.4 branch.
Comment 8 Glenn Randers-Pehrson 2004-07-22 21:13:02 PDT
Created attachment 154063 [details] [diff] [review]
patch for libpng tRNS chunk overflow

I've posted a set of patches for the reported security problems at
http://libpng.sf.net/scary/patches .  Of these, only the one for the
tRNS chunk overflow is relevant to mozilla.  Here is a copy of that
patch.
Comment 9 Glenn Randers-Pehrson 2004-07-26 15:06:20 PDT
Created attachment 154405 [details] [diff] [review]
a more robust fix

This is a more robust fix for the problem, since it works with system libpng
as well as the supplied one.  We wouldn't need the first patch any more, but
neither would it hurt to apply both.
Comment 10 Glenn Randers-Pehrson 2004-07-26 15:08:44 PDT
Taking bug again.  Why does this keep getting reset to NEW?  Maybe
because it is a security bug?
Comment 11 Glenn Randers-Pehrson 2004-07-26 16:49:19 PDT
Re: comment #8 -- I relocated the libpng patches to
http://libpng.sf.net/fix/
and http://libpng.sf.net/fix/patches/
Comment 12 Glenn Randers-Pehrson 2004-07-26 17:25:15 PDT
Comment on attachment 154405 [details] [diff] [review]
a more robust fix

tor: r? (one or both patches)
Comment 13 tor 2004-07-27 13:52:24 PDT
Comment on attachment 154405 [details] [diff] [review]
a more robust fix

Neat fix.  Minor nit: spaces after the commas would be nice,
Comment 14 Glenn Randers-Pehrson 2004-07-27 15:16:44 PDT
Created attachment 154509 [details] [diff] [review]
a more robust fix, with spaces after the commas

Spaces, you got 'em.
Comment 15 tor 2004-07-28 08:39:10 PDT
Glenn: Are you ok with checking in this fix now, or should we keep it
hidden until public disclosure?
Comment 16 chris hofmann 2004-07-28 11:57:07 PDT
google search for png buffer overflow shows quite a bit of public posting on
this bug...
http://www.google.com/search?hl=en&lr=&ie=UTF-8&q=png+buffer+overflow&btnG=Search

seems like it is public but just hasn't drawn a lot of attention yet.
Comment 17 Glenn Randers-Pehrson 2004-07-28 12:13:06 PDT
Chris: They have actually drawn more attention than they really deserve.  Those
are about two different related, fairly harmless bugs (despite the reports
saying they can be exploited).  They don't affect mozilla at all because we
don't operate with 16-bit samples.

Tor: I think wait until August 4th.  CERT is asking for a 6-week extension
beyond that, for the sake of some big vendor, but we are resisting that.

Dunno if the big vendor is Netscape, but the bug blows NS-4.8 away.  I think
it uses an ancient libpng-0.95 or 0.96.

Re comment #8 and comment #11, the patches are now at
http://www.graphicsmagick.org/libpng/beta
Comment 18 Glenn Randers-Pehrson 2004-07-28 18:54:06 PDT
Saw an announcement that NS 7.2 will be out August 3rd.  Will it contain this fix?
Comment 19 chris hofmann 2004-07-30 11:34:34 PDT
we informed netscape a few days ago. haven't heard back from them.  

we need to get the fix to aviary 1.0 and 1.7 branches as soon as possible, so we
can start spinning up mozilla builds for testing and release next week.
Comment 20 Christopher Aillon (sabbatical, not receiving bugmail) 2004-07-30 12:20:50 PDT
This was "disclosed" yesterday on png-implement.  The announcement made
reference to security fixes in the 1.2.6-beta release.  Explicit details were
left out.  We should be able to do the same, I would think....
Comment 21 Glenn Randers-Pehrson 2004-07-30 14:22:14 PDT
Libpng-1.2.6beta4 does not include patches for the most gruesome vulnerabilies,
which includes the one that is addressed by the patches for this bug.
Comment 22 Glenn Randers-Pehrson 2004-08-02 03:59:58 PDT
The PNG development group has been working on a response to the bug report.  We
have concluded that the best way to take care of Item 7 (other flaws) is to
limit the dimensions of PNG files that are accepted.  The PNG spec allows
images up to 2^31-1 by 2^31-1 (i.e., 2.147 billion by 2.147 billion).  We
are implementing a soft limit of 1 million by 1 million that can be overridden
at compile time with a #define or at run time by calling an accessor function.

Tor: should I add that to the patch for this bug, put up a separate patch
here, or start a new bug with a patch?
Comment 23 tor 2004-08-02 04:58:27 PDT
mozilla already has a check to make sure that 4 * width * height doesn't
overflow a 32-bit number.  Is that sufficient to avoid the dimension problem?
Comment 24 Glenn Randers-Pehrson 2004-08-02 08:34:17 PDT
That's not sufficient.  See
http://www.graphicsmagick.org/libpng/beta/samples/bigw.png
which crashes my Firefox but is only 65 million columns by 1 row.
It needs a safety of 64 because it has 64-bit RGBA pixels and there is
arithmetic in libpng that uses the number of bits in a row.

The fix is pretty simple; see *patch11* at
www.graphicsmagick.org/libpng/beta/patches

A slightly more complex fix is in libpng-1.2.6beta4j at
www.graphicsmagick.org/libpng/beta/src
This allows user override of the new 1,000,000 row,column limits.
We don't need that though.

The patch11 has an advantage of quickly rejecting very large valid image
files that mozilla could process without crashing but it takes hours
to do it.

Glenn
Comment 25 tor 2004-08-02 09:01:20 PDT
patch11 looks fine for mozilla, though I'd guess we only really need the
width limit since we're doing progressive reading.  r+sr=tor
Comment 26 tor 2004-08-02 09:06:32 PDT
On second thought, since we may be linked to system libpngs that haven't been
updated, maybe we should to this size limitation in info_callback() after
the call to png_get_IHDR.
Comment 27 Glenn Randers-Pehrson 2004-08-02 09:12:02 PDT
The height limit is also useful, to stop DOS attempts using an image
with a half-billion rows by one column.

About system libraries, imposing the limit in the callback would be OK.
Comment 28 tor 2004-08-02 09:42:12 PDT
Created attachment 154998 [details] [diff] [review]
limit image dimensions, includes tRNS fix
Comment 29 Glenn Randers-Pehrson 2004-08-02 10:49:24 PDT
Here are some CVE names

> 1) Remotely exploitable stack-based buffer overrun in png_handle_tRNS
> (pngrutil.c)
> 2) Dangerous code in png_handle_sBIT (pngrutil.c) (Similar code in
> png_handle_hIST).

CAN-2004-0597 for these (we merge issues that have the same flaw type that 
get fixed in the same versions).

> 3) Possible NULL-pointer crash in png_handle_iCCP (pngrutil.c) (this 
> flaw is duplicated in multiple other locations).

CAN-2004-0598 for those

> 4) Theoretical integer overflow in allocation in png_handle_sPLT 
> (pngrutil.c)
> 5) Integer overflow in png_read_png (pngread.c)
> 6) Integer overflows during progressive reading.
> 7) Other flaws.  [integer overflows]

CAN-2004-0599 for those

The patch looks OK.  It fixes Items 1 and 6 (and maybe 7).  Mozilla is
not vulnerable to Item 5 nor to 2, 3, and 4 when using the embedded libpng.
It might be a good idea to require libpng-1.2.6 once it's out (planned for
August 11), although it's not clear that 2,3,and 4 can really be exploited.
I'll mark the "r" flag this evening after I've built and tested against
some dangerous files.
Comment 30 Glenn Randers-Pehrson 2004-08-02 10:58:10 PDT
Comment on attachment 154998 [details] [diff] [review]
limit image dimensions, includes tRNS fix

r. glennrp
Comment 31 Asa Dotzler [:asa] 2004-08-02 12:54:32 PDT
Adding our release QA team.
Comment 32 sairuh (rarely reading bugmail) 2004-08-02 12:58:39 PDT
is loading http://www.graphicsmagick.org/libpng/beta/samples/bigw.png a
sufficient test for this bug?

fwiw, I loaded that page in seamonkey (linux 2004080114-1.7.2 branch), and no
crash occurred.
Comment 33 Glenn Randers-Pehrson 2004-08-02 14:31:28 PDT
I don't know.  bigw.png crashes my Firefox 0.9.2, leaving my profile "in use"
until I reboot the computer.  I created a talkback item yesterday at 4:02 PM
EST but then lost track of the item number.  If someone can find it, or tell
me where to look on my computer for the ID, it would be helpful to see the
traceback.  I've crashed about 3 or 4 times with that file but only got
into the talkback mode once.
Comment 34 sairuh (rarely reading bugmail) 2004-08-02 15:44:01 PDT
...no crash either, using 2004080211-0.9.3 firefox bits on mac os x 10.3.4.
Comment 35 Glenn Randers-Pehrson 2004-08-02 16:24:55 PDT
Re comment #33, my talkback ID was TB461197W.
Comment 36 Glenn Randers-Pehrson 2004-08-02 16:34:20 PDT
Created attachment 155039 [details] [diff] [review]
patch to skip unused PNG chunks.

The original comments at the scary beasts URL talk about problems with
some ancillary chunks such as sBIT, sPLT, and iCCP.  While we are fixing
those in libpng-1.2.6, it leaves Mozilla applications open to these
problems when loaded with an unpatched older system library.

This patch causes all processing of these unused chunks to be skipped when
the system library is used.

We are running out of time, since cevans plans to go ahead with his
announcement on 4 August at 1400UTC.  I see the other patch is already
checked in.
Comment 37 Glenn Randers-Pehrson 2004-08-02 16:37:43 PDT
Comment on attachment 155039 [details] [diff] [review]
patch to skip unused PNG chunks.

Oops, sRGB shouldn't be on the unused_chunks list.  Let me redo the patch...
Comment 38 sairuh (rarely reading bugmail) 2004-08-02 16:40:01 PDT
the talkback server claims that there's no TB461197W incident. :( nor could I
find reports based on Glenn's email or the test URI from comment 24. unless it's
waiting in a queue somehow...
Comment 39 Glenn Randers-Pehrson 2004-08-02 16:52:50 PDT
Created attachment 155042 [details] [diff] [review]
patch to skip unused PNG chunks (but not sRGB!).

Removed sRGB from the unused-chunks list.
Comment 40 Glenn Randers-Pehrson 2004-08-02 16:54:44 PDT
Comment on attachment 155042 [details] [diff] [review]
patch to skip unused PNG chunks (but not sRGB!).

tor: r?
Comment 41 Christian :Biesinger (don't email me, ping me on IRC) 2004-08-02 17:00:43 PDT
http://talkback-public.mozilla.org/talkback/fastfind.jsp claims that the current
talkback queue is rather long...
Comment 42 tor 2004-08-02 17:44:45 PDT
Comment on attachment 155042 [details] [diff] [review]
patch to skip unused PNG chunks (but not sRGB!).

'b', 'K', 'G', 'D' etc. might have been a bit easier to read,
but the table checks out.
Comment 43 Glenn Randers-Pehrson 2004-08-02 18:10:20 PDT
The PNG spec cautions us to use integers, not characters, to build the chunk
types.  Otherwise you're hosed if you happen to use a compiler with a localized
character set.  I never thought there was a lot of danger of that, but there you
have it.  The table works out OK because I just cut-and-pasted it out of png.h.
Comment 44 Glenn Randers-Pehrson 2004-08-03 10:39:45 PDT
Created attachment 155093 [details] [diff] [review]
patch to skip unused PNG chunks (2)

There were a couple of bugs in my previous patch.  Missing ";" at the
end of the unused chunks list, and an extra parameter "mInfo" in the
call to png_set_keep_unknown_chunks().
Comment 45 Glenn Randers-Pehrson 2004-08-03 10:41:11 PDT
Comment on attachment 155093 [details] [diff] [review]
patch to skip unused PNG chunks (2)

Tor: r?
Comment 46 tor 2004-08-03 10:56:43 PDT
Comment on attachment 155093 [details] [diff] [review]
patch to skip unused PNG chunks (2)

You'll fix libpng.txt in the next libpng release to remove the
documentation of the extra parameter (still mentioned in rc5)?
r=tor
Comment 47 Glenn Randers-Pehrson 2004-08-03 11:43:34 PDT
libpng documentation fixed in version rc6.  Thanks, I couldn't figure why
I had included the extra param.

We want to think about installing libpng-1.2.6.  The final "rc" version will
be posted tomorrow and released as libpng-1.2.6 on August 11.  We might want
to let it simmer for another week or so before installing it here.  It
fixes a few more security issues that our patches don't fix, but they don't
look terribly serious.
Comment 48 tor 2004-08-03 11:51:13 PDT
Once libpng 1.2.6 has been out a bit, I was thinking of updating the mozilla
tree version and the minimum required version.  Most of these fixes can come
out at the same time, save possibly the dimension limit.

Branches will stay at 1.2.5 + these fixes.
Comment 49 Christopher Aillon (sabbatical, not receiving bugmail) 2004-08-03 13:24:09 PDT
Comment on attachment 155093 [details] [diff] [review]
patch to skip unused PNG chunks (2)

a=blizzard for 1.4.3
Comment 50 Christopher Aillon (sabbatical, not receiving bugmail) 2004-08-03 13:25:19 PDT
Comment on attachment 154998 [details] [diff] [review]
limit image dimensions, includes tRNS fix

a=blizzard for 1.4.3
Comment 51 tor 2004-08-03 14:09:14 PDT
Created attachment 155114 [details] [diff] [review]
combined set of fixes

Combine the two patches and remove the "const" in the unused_chunks 
declaration that was causing compile problems.
Comment 52 Mike Kaply [:mkaply] (Out June 27-July 5) 2004-08-04 06:48:42 PDT
Comment on attachment 155114 [details] [diff] [review]
combined set of fixes

a=mkaply for both
Comment 53 tor 2004-08-04 07:04:23 PDT
Checked into trunk, MOZILLA_1_7_2_BRANCH, MOZILLA_1_4_BRANCH,
MOZILLA_1_7_BRANCH, AVIARY_1_0_20040515_BRANCH.

Past scheduled public disclosure, removing security flag.
Comment 54 Glenn Randers-Pehrson 2004-08-04 08:43:46 PDT
Firefox 0.9.3 seems to have no problem with my sample bad images.
GECKO 20040803/Firefox 0.9.3
The installer thinks it's installing 0.9.2 though.

verified fixed.
Comment 55 sairuh (rarely reading bugmail) 2004-08-04 09:37:18 PDT
(In reply to comment #54)
> Firefox 0.9.3 seems to have no problem with my sample bad images.
> GECKO 20040803/Firefox 0.9.3
> The installer thinks it's installing 0.9.2 though.

side note: the installer issue is bug 254097.

Glenn, thanks for verifying this (as well as the testing and patches)!

Comment 56 Marc Bejarano 2004-08-06 19:16:07 PDT
this should have been relnoted for Firefox.  see bug 254655.
Comment 57 David Epstein 2004-08-12 13:16:47 PDT
Created attachment 155971 [details]
bad png image for test case

This image shouldn't open. When trying to open, it should give msg "the image
pngtest_bad.png cannot be displayed, because it contains errors."
Comment 58 Matthias Versen [:Matti] 2004-09-01 01:16:56 PDT
*** Bug 257492 has been marked as a duplicate of this bug. ***

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