334110 - Upgrade to libpng-1.2.12

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, enhancement)

Description

[Manfred H. Winter]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20060415 Firefox/2.0a1 (mahowi)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20060415 Firefox/2.0a1 (mahowi)

Mozilla still uses libpng-1.2.7 although there are newer versions available. Yesterday 1.2.9 got released.

Reproducible: Always

Comment 1

[Manfred H. Winter]

Attachment: update to libpng-1.2.9

Comment 2

[Glenn Randers-Pehrson]

libpng-1.2.10 will be out in a few days to correct a problem with the "configure" script.

Comment 3

[Glenn Randers-Pehrson]

Taking bug.  libpng-1.2.10 should be ready within a week.

Comment 4

[Manfred H. Winter]

Okay, I'll upload a new patch then.

Comment 5

[tor]

Glenn: are there changes to libpng since 1.2.7 that make this something we should take in the FF2 branch?

Comment 6

[Glenn Randers-Pehrson]

tor: There isn't much change to the libpng code since 1.2.7, but there is some opportunity to save a few kbytes of footprint, especially in a situation where the PNG decoder is disabled but the PNG encoder is being used (I'm not sure that is the case with FF2 but can be true on the trunk).  I'll post a revised mozpngconf.h that mahowi can merge with the patch.

Comment 7

[Glenn Randers-Pehrson]

Attachment: New png/Makefile.in, mozpngconf.h, and configure update

For use with libpng-1.2.10

Comment 8

[Manfred H. Winter]

Attachment: libpng-1.2.10 patch for branch

Comment 9

[Manfred H. Winter]

Attachment: libpng-1.2.10 patch for trunk

Comment 10

[Glenn Randers-Pehrson]

Comment on attachment 219509 [details] [diff] [review]
libpng-1.2.10 patch for branch

The patches look OK to me and WFM.  tor: r?

Comment 11

[tor]

Comment on attachment 219509 [details] [diff] [review]
libpng-1.2.10 patch for branch

Too late for branch, I think.

Comment 12

[rbs]

Comment on attachment 219511 [details] [diff] [review]
libpng-1.2.10 patch for trunk

Try somebody else, perhaps blizzard or shaver, for the sr.
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/modules/libimg/png/MOZCHANGES&rev=HEAD&mark=3.12

Comment 13

[Christopher Blizzard (:blizzard)]

I'm not doing reviews anymore.  You will have to find someone else to review it.  Sorry!

Comment 14

[Glenn Randers-Pehrson]

Comment on attachment 219509 [details] [diff] [review]
libpng-1.2.10 patch for branch

The "branch" patch was rendered obsolete by checkin of bug #338407.  Now, simply use the "trunk" patch to update either the trunk or the branch 1.8.

Comment 15

[Glenn Randers-Pehrson]

Libpng-1.2.11 will be released in the next two weeks or so.  The only code change, to eliminate a potential minor buffer-overflow, is in a part of pngrutil.c that is #ifdef'ed out of the embedded library.

Comment 16

[Glenn Randers-Pehrson]

Attachment: Upgrade trunk to libpng-1.2.11

Libpng-1.2.11 has been released.  It fixes two potential buffer overruns and one out-of-bounds read.

Comment 17

[Glenn Randers-Pehrson]

Comment on attachment 227078 [details]
Upgrade trunk to libpng-1.2.11

The buffer-overflow bug in error processing is still in libpng-1.2.11.

Comment 18

[Glenn Randers-Pehrson]

Attachment: Update trunk with libpng-1.2.12 (bz2) (checked in)

Libpng-1.2.12 has been released, to fix a potential buffer overrun in chunk error processing.

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 227318 [details]
Update trunk with libpng-1.2.12 (bz2) (checked in)

Whoops, I totally missed this review request, sorry :(  Stuart is the right person for anything imagelib-related, bouncing to him.

Comment 20

[Stuart Parmenter]

Comment on attachment 227318 [details]
Update trunk with libpng-1.2.12 (bz2) (checked in)

lets get this in

Comment 21

[Glenn Randers-Pehrson]

Comment on attachment 227318 [details]
Update trunk with libpng-1.2.12 (bz2) (checked in)

tor: sr?

Comment 22

[Glenn Randers-Pehrson]

Would someone please check this in to the trunk and 1.8 branch?

Comment 23

[Reed Loden [:reed]]

(In reply to comment #22)
> Would someone please check this in to the trunk and 1.8 branch?

vlad mentioned on IRC that he would get stuart to check it in sometime today.

Comment 24

[tor]

(In reply to comment #22)
> Would someone please check this in to the trunk and 1.8 branch?

Too late for 1.8 branch.

Comment 25

[Glenn Randers-Pehrson]

I didn't think it was ever too late to fix security problems.

Comment 26

[tor]

(In reply to comment #25)
> I didn't think it was ever too late to fix security problems.

We're in release candidate stage for 1.8.1 - too late to fix security problems by doing a large update of an external library.  If you have smaller security specific patches to the version of libpng on the branch we'd be interested in hearing about them.

Comment 27

[Glenn Randers-Pehrson]

Attachment: Fix libpng-1.2.7 security problems on 1.8 branch (checked in on branch)

Patch to fix only the security problems in libpng-1.2.7 and the libpr0n png decoder, on the 1.8 branch.

Comment 28

[Stuart Parmenter]

landed 1.2.12 on the trunk. leaving open for possible branch landing

Comment 29

[Stuart Parmenter]

backed out due to mac bustage i'm not sure how to fix without more time.

Comment 30

[Daniel Veditz [:dveditz]]

If you get a patch that works for FF2 we'll want it for 1.5.0.x also.

Comment 31

[Daniel Veditz [:dveditz]]

The vulnerabilities are announced at www.libpng.org, so we might want to push for FF2 rather than 1.8.1.1 (although as overruns go they don't sound so bad).

Glenn: can you be more specific about the sCAL write overrun? "rare" in real life images is not the same thing as "hard to do" when writing an exploit. Can a malicious image control the amount of overwrite here?

Comment 32

[Glenn Randers-Pehrson]

The sCAL bug cannot affect Firefox or any Gecko-based software.  It only affects applications that deliberately write the sCAL chunk, i.e., that contain a call to png_write_sCAL() or png_write_sCAL_s().  libpr0n contains no such calls.

The buffer error message overrun is just two bytes and they are not under the
control of the image creator.  The gamma_table overrun is just one byte being
read beyond an array and probably also not under the control of an image
creator.  There aren't any known exploits for these.

On the other hand, it is simple to construct a PNG file with a malformed iCCP chunk that could crash Gecko when the system PNG library is employed.  The fix
in nsPNGDecoder.cpp avoids that and similar problems by always handling iCCP
as an "unknown chunk to be ignored".

Comment 33

[Glenn Randers-Pehrson]

What is the nature of the mac bustage?

Comment 34

[Glenn Randers-Pehrson]

It may take some time to track down and fix the mac bustage.  How about applying the small security-only fix to the trunk, for now?

Comment 35

[Mike Schroepfer]

Yes - let's get the smaller security patch on trunk asap...

Comment 36

[Glenn Randers-Pehrson]

Comment on attachment 227318 [details]
Update trunk with libpng-1.2.12 (bz2) (checked in)

marking the big batch obsolete until the mac problem is fixed.

Comment 37

[Mike Schroepfer]

Comment on attachment 239681 [details] [diff] [review]
Fix libpng-1.2.7 security problems on 1.8 branch (checked in on branch)

Let get this in 1.8.1 branch as well.  Approved for RC2 - thanks Glenn!

Comment 38

[Daniel Veditz [:dveditz]]

Comment on attachment 239681 [details] [diff] [review]
Fix libpng-1.2.7 security problems on 1.8 branch (checked in on branch)

approved for 1.8.0 branch, a=dveditz for drivers

Comment 39

[Stuart Parmenter]

checked in on branch and trunk

Comment 40

[Daniel Veditz [:dveditz]]

Comment on attachment 239681 [details] [diff] [review]
Fix libpng-1.2.7 security problems on 1.8 branch (checked in on branch)

approved for 1.8.0 branch, a=dveditz for drivers

Comment 41

[Glenn Randers-Pehrson]

Should we leave the bug open for fixing the mac bustage, or start a new one?  It's not urgent now that the security is tightened up.

Comment 42

[Stuart Parmenter]

should probably leave this open for the mac bustage on the big patch -- we want to get apng in soon and on libpng 1.2.12.  If anything, we should have probably filed another bug for the security patch.  Oh well, too late now.

Comment 43

[Andrew Smith]

the 1.2.12 patch has a new configure.in but configure was not regenerated (as far as i can tell). i'm pretty sure this was causing the build problems, will test on a mac tomorrow.

Comment 44

[Stuart Parmenter]

configure gets regenerated automatically whenever configure.in is changed in cvs.  the change did get picked up on the tinderboxes -- you can see the checkins on bonsai from when I landed.

Comment 45

[Glenn Randers-Pehrson]

Pav: can you post details of the "mac bustage" either here or (if the problem seems to be in libpng) on the png-mng-implement(at)lists.sf.net mailing list?

Comment 46

[Stuart Parmenter]

It was just a bunch of undefined symbols.  I'll post the exact ones shortly.

Comment 47

[Stuart Parmenter]

checked in to the 1.8.0 branch as well (which i think is the fixed1.8.0.8 kw..)

Comment 48

[Stuart Parmenter]

When I try to build with the bigger patch on the mac I end up getting:

/usr/bin/ld: Undefined symbols:
_MOZ_PNG_mmx_support
_MOZ_PNG_combine_row
_MOZ_PNG_do_read_int
_MOZ_PNG_read_filt_row
collect2: ld returned 1 exit status

Comment 49

[Glenn Randers-Pehrson]

Pav:  try adding pnggccrd.c to the list of source files in libimg/png/Makefile.in

Comment 50

[Sylvain Pasche]

Looks like it broke x86_64 builds:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1159571460.22166.gz

Comment 51

[Glenn Randers-Pehrson]

To avoid the x86_64 problem, we could put in mozlibpngconf.h

#define PNG_NO_MMX_CODE

Doing it conditionally (so as now to slow down regular x86 machines)
is more complicated.  We address it in libpng-1.4.0 by doing a trial
compile of pnggccrd.c in the configure script and setting the #define
if it fails.

Comment 52

[Glenn Randers-Pehrson]

To avoid the x86_64 problem, we could put in mozlibpngconf.h

#define PNG_NO_MMX_CODE

Doing it conditionally (so as not to slow down regular x86 machines)
is more complicated.  We address it in libpng-1.4.0 by doing a trial
compile of pnggccrd.c in the configure script and setting the #define
if it fails.

Comment 53

[Glenn Randers-Pehrson]

I've opened bug #354966 about the x86_64 compiling problem.

Comment 54

[Glenn Randers-Pehrson]

The file libimg/png/pngasmrd.h is left over from an old libpng version and should be removed.

Comment 55

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This patch didn't change the minimum required system libpng in configure (for those who use -with-system-libpng) to match what was put in the tree, but it probably should.  (I might clean that up in bug 372878 if nobody else does first.)