Closed Bug 334110 Opened 14 years ago Closed 13 years ago

Upgrade to libpng-1.2.12

Categories

(Core :: ImageLib, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mahowi, Assigned: glennrp+bmo)

References

Details

(Keywords: fixed1.8.0.8, fixed1.8.1)

Attachments

(2 files, 5 obsolete files)

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
Attached patch update to libpng-1.2.9 (obsolete) — Splinter Review
Attachment #218515 - Flags: review?(glennrp)
libpng-1.2.10 will be out in a few days to correct a problem with the "configure" script.
Assignee: pavlov → glennrp
Status: UNCONFIRMED → NEW
Ever confirmed: true
Taking bug.  libpng-1.2.10 should be ready within a week.
Status: NEW → ASSIGNED
Okay, I'll upload a new patch then.
Glenn: are there changes to libpng since 1.2.7 that make this something we should take in the FF2 branch?
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.
Attachment #218515 - Attachment is obsolete: true
Attachment #218515 - Flags: review?(glennrp)
For use with libpng-1.2.10
Attached patch libpng-1.2.10 patch for branch (obsolete) — Splinter Review
Attachment #219509 - Flags: review?(glennrp)
Attached patch libpng-1.2.10 patch for trunk (obsolete) — Splinter Review
Attachment #219511 - Flags: review?(glennrp)
Attachment #219371 - Attachment is obsolete: true
Comment on attachment 219509 [details] [diff] [review]
libpng-1.2.10 patch for branch

The patches look OK to me and WFM.  tor: r?
Attachment #219509 - Flags: review?(glennrp) → review?(tor)
Attachment #219511 - Flags: review?(glennrp) → review?(tor)
Summary: newer libpng available than 1.2.7 → Upgrade to libpng-1.2.10
Attachment #219511 - Flags: review?(tor) → review+
Comment on attachment 219509 [details] [diff] [review]
libpng-1.2.10 patch for branch

Too late for branch, I think.
Attachment #219509 - Flags: review?(tor) → review-
Attachment #219511 - Flags: superreview?(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
Attachment #219511 - Flags: superreview?(rbs)
Attachment #219511 - Flags: superreview?(blizzard)
I'm not doing reviews anymore.  You will have to find someone else to review it.  Sorry!
Attachment #219511 - Flags: superreview?(blizzard)
Blocks: 191033
Blocks: 171082
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.
Attachment #219509 - Attachment is obsolete: true
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.
Attached file Upgrade trunk to libpng-1.2.11 (obsolete) —
Libpng-1.2.11 has been released.  It fixes two potential buffer overruns and one out-of-bounds read.
Attachment #219511 - Attachment is obsolete: true
Attachment #227078 - Flags: review?(vladimir)
Summary: Upgrade to libpng-1.2.10 → Upgrade to libpng-1.2.11
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.
Attachment #227078 - Flags: review?(vladimir)
Libpng-1.2.12 has been released, to fix a potential buffer overrun in chunk error processing.
Attachment #227078 - Attachment is obsolete: true
Attachment #227318 - Flags: review?(vladimir)
Summary: Upgrade to libpng-1.2.11 → Upgrade to libpng-1.2.12
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.
Attachment #227318 - Flags: review?(vladimir) → review?(pavlov)
Comment on attachment 227318 [details]
Update trunk with libpng-1.2.12 (bz2) (checked in)

lets get this in
Attachment #227318 - Flags: review?(pavlov) → review+
Comment on attachment 227318 [details]
Update trunk with libpng-1.2.12 (bz2) (checked in)

tor: sr?
Attachment #227318 - Flags: superreview?(tor)
Attachment #227318 - Flags: superreview?(tor) → superreview+
Whiteboard: [checkin needed]
Would someone please check this in to the trunk and 1.8 branch?
(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.
(In reply to comment #22)
> Would someone please check this in to the trunk and 1.8 branch?

Too late for 1.8 branch.
I didn't think it was ever too late to fix security problems.
(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.
Patch to fix only the security problems in libpng-1.2.7 and the libpr0n png decoder, on the 1.8 branch.
Attachment #239681 - Flags: review?(pavlov)
Flags: blocking1.8.1?
Flags: blocking1.8.1.1?
landed 1.2.12 on the trunk. leaving open for possible branch landing
backed out due to mac bustage i'm not sure how to fix without more time.
If you get a patch that works for FF2 we'll want it for 1.5.0.x also.
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.8?
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?
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".
What is the nature of the mac bustage?
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?
Yes - let's get the smaller security patch on trunk asap...
Flags: blocking1.8.1? → blocking1.8.1+
Attachment #239681 - Flags: superreview?(tor)
Attachment #239681 - Flags: review?(pavlov)
Attachment #239681 - Flags: review+
Attachment #239681 - Flags: superreview?(tor) → superreview+
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.
Attachment #227318 - Attachment is obsolete: true
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!
Attachment #239681 - Flags: approval1.8.1+
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8+
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
checked in on branch and trunk
Keywords: fixed1.8.1
Whiteboard: [checkin needed]
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
Attachment #239681 - Flags: approval1.8.0.8+
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.
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.
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.
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.
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?
It was just a bunch of undefined symbols.  I'll post the exact ones shortly.
checked in to the 1.8.0 branch as well (which i think is the fixed1.8.0.8 kw..)
Keywords: fixed1.8.0.8
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
Pav:  try adding pnggccrd.c to the list of source files in libimg/png/Makefile.in
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.
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.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I've opened bug #354966 about the x86_64 compiling problem.
Depends on: 354997
Depends on: 354966
The file libimg/png/pngasmrd.h is left over from an old libpng version and should be removed.
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.)
Attachment #227318 - Attachment description: Update trunk with libpng-1.2.12 (bz2) → Update trunk with libpng-1.2.12 (bz2) (checked in)
Attachment #227318 - Attachment is obsolete: false
Attachment #239681 - Attachment description: Fix libpng-1.2.7 security problems on 1.8 branch → Fix libpng-1.2.7 security problems on 1.8 branch (checked in on branch)
QA Contact: imagelib
You need to log in before you can comment on or make changes to this bug.