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
Should we apply the patch from your report to our in-tree copy of libpng?
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.
13 years ago
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.
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.
assigning to Glenn per comment 4
what should the release plans be for this? should get patched on trunk, 1.7 branch and aviary 1.0 branch.
and 1.4 branch.
13 years ago
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.
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.
Taking bug again. Why does this keep getting reset to NEW? Maybe because it is a security bug?
Re: comment #8 -- I relocated the libpng patches to http://libpng.sf.net/fix/ and http://libpng.sf.net/fix/patches/
Comment on attachment 154405 [details] [diff] [review] a more robust fix tor: r? (one or both patches)
Comment on attachment 154405 [details] [diff] [review] a more robust fix Neat fix. Minor nit: spaces after the commas would be nice,
Created attachment 154509 [details] [diff] [review] a more robust fix, with spaces after the commas Spaces, you got 'em.
Glenn: Are you ok with checking in this fix now, or should we keep it hidden until public disclosure?
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.
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
Saw an announcement that NS 7.2 will be out August 3rd. Will it contain this fix?
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.
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....
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.
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?
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?
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
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
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.
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.
Created attachment 154998 [details] [diff] [review] limit image dimensions, includes tRNS fix
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 on attachment 154998 [details] [diff] [review] limit image dimensions, includes tRNS fix r. glennrp
Adding our release QA team.
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.
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.
...no crash either, using 2004080211-0.9.3 firefox bits on mac os x 10.3.4.
Re comment #33, my talkback ID was TB461197W.
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 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...
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...
Created attachment 155042 [details] [diff] [review] patch to skip unused PNG chunks (but not sRGB!). Removed sRGB from the unused-chunks list.
Comment on attachment 155042 [details] [diff] [review] patch to skip unused PNG chunks (but not sRGB!). tor: r?
http://talkback-public.mozilla.org/talkback/fastfind.jsp claims that the current talkback queue is rather long...
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.
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.
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 on attachment 155093 [details] [diff] [review] patch to skip unused PNG chunks (2) Tor: r?
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
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.
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 on attachment 155093 [details] [diff] [review] patch to skip unused PNG chunks (2) a=blizzard for 1.4.3
Comment on attachment 154998 [details] [diff] [review] limit image dimensions, includes tRNS fix a=blizzard for 1.4.3
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.
13 years ago
Comment on attachment 155114 [details] [diff] [review] combined set of fixes a=mkaply for both
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.
13 years ago
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.
(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)!
this should have been relnoted for Firefox. see bug 254655.
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."
*** Bug 257492 has been marked as a duplicate of this bug. ***