new libpng buffer overflow vulnerabilities

VERIFIED FIXED

Status

()

Core
ImageLib
--
major
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: Glenn Randers-Pehrson, Assigned: Glenn Randers-Pehrson)

Tracking

(4 keywords)

Trunk
fixed-aviary1.0, fixed1.4.3, fixed1.7.2, fixed1.7.5
Points:
---
Bug Flags:
blocking1.7.5 +
blocking-aviary1.0PR +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix]blocking1.4.3+, URL)

Attachments

(2 attachments, 7 obsolete attachments)

(Assignee)

Description

13 years ago
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?
(Assignee)

Comment 2

13 years ago
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.
Whiteboard: blocking1.4.3+
(Assignee)

Comment 3

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.
(Assignee)

Comment 4

13 years ago
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.
Status: NEW → ASSIGNED
assigning to Glenn per comment 4
Flags: blocking1.8a2?
Flags: blocking1.7.2?
Flags: blocking-aviary1.0RC1?
Whiteboard: blocking1.4.3+ → [sg:fix]blocking1.4.3+

Comment 6

13 years ago
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.
Flags: blocking1.7.2?
Flags: blocking1.7.2+
Flags: blocking-aviary1.0RC1?
Flags: blocking-aviary1.0RC1+
(Assignee)

Comment 8

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.

Updated

13 years ago
Assignee: jdunn → glennrp
Status: ASSIGNED → NEW
(Assignee)

Comment 9

13 years ago
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.
(Assignee)

Comment 10

13 years ago
Taking bug again.  Why does this keep getting reset to NEW?  Maybe
because it is a security bug?
Status: NEW → ASSIGNED
(Assignee)

Comment 11

13 years ago
Re: comment #8 -- I relocated the libpng patches to
http://libpng.sf.net/fix/
and http://libpng.sf.net/fix/patches/
(Assignee)

Comment 12

13 years ago
Comment on attachment 154405 [details] [diff] [review]
a more robust fix

tor: r? (one or both patches)
Attachment #154405 - Flags: review?(tor)

Comment 13

13 years ago
Comment on attachment 154405 [details] [diff] [review]
a more robust fix

Neat fix.  Minor nit: spaces after the commas would be nice,
Attachment #154405 - Flags: superreview+
Attachment #154405 - Flags: review?(tor)
Attachment #154405 - Flags: review+
(Assignee)

Comment 14

13 years ago
Created attachment 154509 [details] [diff] [review]
a more robust fix, with spaces after the commas

Spaces, you got 'em.
Attachment #154063 - Attachment is obsolete: true
Attachment #154405 - Attachment is obsolete: true

Comment 15

13 years ago
Glenn: Are you ok with checking in this fix now, or should we keep it
hidden until public disclosure?

Comment 16

13 years ago
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.
(Assignee)

Comment 17

13 years ago
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
(Assignee)

Comment 18

13 years ago
Saw an announcement that NS 7.2 will be out August 3rd.  Will it contain this fix?

Comment 19

13 years ago
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....
(Assignee)

Comment 21

13 years ago
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.
(Assignee)

Comment 22

13 years ago
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

13 years ago
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?
(Assignee)

Comment 24

13 years ago
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

13 years ago
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

13 years ago
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.
(Assignee)

Comment 27

13 years ago
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

13 years ago
Created attachment 154998 [details] [diff] [review]
limit image dimensions, includes tRNS fix
Attachment #154509 - Attachment is obsolete: true

Updated

13 years ago
Attachment #154998 - Flags: review?(glennrp)
(Assignee)

Comment 29

13 years ago
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.
(Assignee)

Comment 30

13 years ago
Comment on attachment 154998 [details] [diff] [review]
limit image dimensions, includes tRNS fix

r. glennrp
Attachment #154998 - Flags: review?(glennrp) → review+

Comment 31

13 years ago
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.
(Assignee)

Comment 33

13 years ago
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.
(Assignee)

Comment 35

13 years ago
Re comment #33, my talkback ID was TB461197W.
(Assignee)

Comment 36

13 years ago
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.
(Assignee)

Comment 37

13 years ago
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...
Attachment #155039 - Attachment is obsolete: true
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...
(Assignee)

Comment 39

13 years ago
Created attachment 155042 [details] [diff] [review]
patch to skip unused PNG chunks (but not sRGB!).

Removed sRGB from the unused-chunks list.
(Assignee)

Comment 40

13 years ago
Comment on attachment 155042 [details] [diff] [review]
patch to skip unused PNG chunks (but not sRGB!).

tor: r?
Attachment #155042 - Flags: review?(tor)
http://talkback-public.mozilla.org/talkback/fastfind.jsp claims that the current
talkback queue is rather long...

Comment 42

13 years ago
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.
Attachment #155042 - Flags: review?(tor) → review+
(Assignee)

Comment 43

13 years ago
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.

Updated

13 years ago
Attachment #154998 - Flags: approval1.7.3?
Attachment #154998 - Flags: approval1.4.3?
Attachment #154998 - Flags: approval-aviary?

Updated

13 years ago
Attachment #155042 - Flags: approval1.7.3?
Attachment #155042 - Flags: approval1.4.3?
Attachment #155042 - Flags: approval-aviary?
(Assignee)

Comment 44

13 years ago
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().
Attachment #155042 - Attachment is obsolete: true
(Assignee)

Comment 45

13 years ago
Comment on attachment 155093 [details] [diff] [review]
patch to skip unused PNG chunks (2)

Tor: r?
Attachment #155093 - Flags: review?(tor)

Comment 46

13 years ago
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
Attachment #155093 - Flags: review?(tor)
Attachment #155093 - Flags: review+
Attachment #155093 - Flags: approval1.7.3?
Attachment #155093 - Flags: approval1.4.3?
Attachment #155093 - Flags: approval-aviary?

Updated

13 years ago
Attachment #155042 - Flags: approval1.7.3?
Attachment #155042 - Flags: approval1.4.3?
Attachment #155042 - Flags: approval-aviary?
(Assignee)

Comment 47

13 years ago
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

13 years ago
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
Attachment #155093 - Flags: approval1.4.3? → approval1.4.3+
Comment on attachment 154998 [details] [diff] [review]
limit image dimensions, includes tRNS fix

a=blizzard for 1.4.3
Attachment #154998 - Flags: approval1.4.3? → approval1.4.3+

Comment 51

13 years ago
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.
Attachment #154998 - Attachment is obsolete: true
Attachment #155093 - Attachment is obsolete: true

Updated

13 years ago
Attachment #154998 - Flags: approval1.7.3?
Attachment #154998 - Flags: approval-aviary?

Updated

13 years ago
Attachment #155093 - Flags: approval1.7.3?
Attachment #155093 - Flags: approval-aviary?
Keywords: fixed1.4.3

Updated

13 years ago
Attachment #155114 - Flags: approval1.7.3?
Attachment #155114 - Flags: approval-aviary?

Comment 52

13 years ago
Comment on attachment 155114 [details] [diff] [review]
combined set of fixes

a=mkaply for both
Attachment #155114 - Flags: approval1.7.3?
Attachment #155114 - Flags: approval1.7.3+
Attachment #155114 - Flags: approval-aviary?
Attachment #155114 - Flags: approval-aviary+

Comment 53

13 years ago
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.
Group: security
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Keywords: fixed-aviary1.0, fixed1.7.2, fixed1.7.3
(Assignee)

Comment 54

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.
Status: RESOLVED → VERIFIED
(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

13 years ago
this should have been relnoted for Firefox.  see bug 254655.

Comment 57

13 years ago
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."
Flags: blocking1.8a2?
*** Bug 257492 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.