Update trunk to libpng-1.2.31

RESOLVED FIXED

Status

()

Core
ImageLib
RESOLVED FIXED
9 years ago
8 years ago

People

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

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 12 obsolete attachments)

22.45 KB, image/png
Details
209.24 KB, patch
Details | Diff | Splinter Review
353.04 KB, patch
Joe Drew (not getting mail)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
Libpng-1.2.25 has been released.  It fixes several NULL dereferences found in the library (not necessarily in Mozilla's embedded libpng) by Coverity.  It does not seem likely that any of these could be exploited remotely.  Libpng-1.2.25 does not introduce any new features.
(Assignee)

Comment 1

9 years ago
Created attachment 304823 [details] [diff] [review]
Update trunk to libpng-1.2.25
Attachment #304823 - Flags: review?(tor)
FYI, the attached patch appears to have Windows line endings.
(Assignee)

Comment 3

9 years ago
Created attachment 305119 [details] [diff] [review]
Update trunk to libpng-1.2.25 (v1)

Now with 1-byte line endings
Attachment #304823 - Attachment is obsolete: true
Attachment #305119 - Flags: review?(tor)
Attachment #304823 - Flags: review?(tor)
Just wanted to mention that I've been running with this for the last week without any issues thus far.
(Assignee)

Comment 5

9 years ago
Libpng-1.2.26 is in process but I don't believe it has any implications yet
for mozilla.  I guess it will be out around the end of March.
(Assignee)

Comment 6

9 years ago
Created attachment 313392 [details] [diff] [review]
update trunk to libpng-1.2.26

Libpng-1.2.26 has been released.
Attachment #305119 - Attachment is obsolete: true
Attachment #305119 - Flags: review?(tor)
Summary: Update trunk to libpng-1.2.25 → Update trunk to libpng-1.2.26
(Assignee)

Updated

9 years ago
Attachment #313392 - Flags: review?(tor)
(Assignee)

Comment 7

9 years ago
Created attachment 314396 [details] [diff] [review]
Update trunk to libpng-1.2.26 (v1)

Resolves merge conflict due to checkin of patch from bug #420416
Attachment #313392 - Attachment is obsolete: true
Attachment #314396 - Flags: review?(pavlov)
Attachment #313392 - Flags: review?(tor)
(Assignee)

Comment 8

9 years ago
Libpng-1.2.27beta01 was released this morning (April 12) and libpng-1.2.27 is scheduled for release on or about April 26, 2008.
(Assignee)

Comment 9

9 years ago
I recommend skipping 1.2.26 and waiting for libpng-1.2.27.  This is expected to resolve bug #428045
Blocks: 428045
Severity: trivial → normal
Summary: Update trunk to libpng-1.2.26 → Update trunk to libpng-1.2.27
(Assignee)

Updated

9 years ago
Attachment #314396 - Attachment is obsolete: true
Attachment #314396 - Flags: review?(pavlov)
(Assignee)

Comment 10

9 years ago
Created attachment 315292 [details] [diff] [review]
update trunk to libpng-1.2.27beta01 (for testing)

This patch updates the trunk to libpng-1.2.27beta01.  I'll replace the patch with a new one and request r/sr when libpng-1.2.27 is released on or about April 26, 2008.  This should fix bug #428045.
(Assignee)

Updated

9 years ago
No longer blocks: 428045
(Assignee)

Comment 11

9 years ago
Comment on attachment 315292 [details] [diff] [review]
update trunk to libpng-1.2.27beta01 (for testing)

Marking patch obsolete because the libpng group has decided to take another approach (simply issuing a warning without fixing the erroneous tRNS chunk data).  I'll post a new patch after the dust settles.
Attachment #315292 - Attachment is obsolete: true
(Assignee)

Comment 12

9 years ago
Created attachment 316078 [details] [diff] [review]
Update trunk to libpng-1.2.27beta03 (for testing)

Here is a preview of libpng-1.2.27 which will be released around April 30.
(Assignee)

Comment 13

9 years ago
Created attachment 318302 [details] [diff] [review]
Update trunk to libpng-1.2.27

Libpng-1.2.27 has been released.
Attachment #316078 - Attachment is obsolete: true
Attachment #318302 - Flags: review?(pavlov)
(Assignee)

Comment 14

9 years ago
Libpng-1.2.28 has been released.  The libpng code is exactly the
same as in libpng-1.2.27; there is only a change in configuration
files that we do not use here, so there seems to be no need for
a new patch.
(Assignee)

Comment 15

9 years ago
Created attachment 320262 [details] [diff] [review]
Update trunk to libpng-1.2.29

Libpng-1.2.29 has been released.
Attachment #318302 - Attachment is obsolete: true
Attachment #320262 - Flags: review?(tor)
Attachment #318302 - Flags: review?(pavlov)
(Assignee)

Updated

9 years ago
Summary: Update trunk to libpng-1.2.27 → Update trunk to libpng-1.2.29

Comment 16

9 years ago
Created attachment 320493 [details]
image that fails to decode

Assuming the 1.2.29 patch is the same as the 1.2.27 patch, it has a broken sequencial APNG reader.

Mostly it works, and the only thing I can think of that's special about this image is that the second frame is smaller than the first. Reading the second frame fails with:

...
        in png_read_row (row 71, pass 0)
        in png_combine_row
        in png_read_finish_row
        in png_read_row (row 72, pass 0)
        in png_combine_row
        in png_read_finish_row
        in png_read_row (row 73, pass 0)
libpng error: Extra compressed data

The sequencial reader isn't being used in Mozilla, but it probably shouldn't be included at all if it's broken.

I'm looking into it, but have very little spare time.

Comment 17

9 years ago
The problem goes away if the code in png_read_reset() is altered to be:

    png_ptr->flags &= ~PNG_FLAG_ROW_INIT;
    //png_read_start_row(png_ptr);

instead of the other way around.
(Assignee)

Comment 18

9 years ago
Created attachment 320539 [details] [diff] [review]
Update trunk to libpng-1.2.29 (v1)

Revised patch per Andrew's comment.
Attachment #320262 - Attachment is obsolete: true
Attachment #320539 - Flags: review?(tor)
Attachment #320262 - Flags: review?(tor)

Comment 19

9 years ago
Sadly, it looks like with that the image in question works but several more others no longer do.
(Assignee)

Comment 20

9 years ago
(In reply to comment #19)
> Sadly, it looks like with that the image in question works but several more
> others no longer do.

Is that with the progressive reader (libimg/libpr0n) or with the sequential
reader?  Is the nature of the error the same (wrong portions of the image
visible on subsequent frames)?

(Assignee)

Updated

9 years ago
Blocks: 433047
(Assignee)

Comment 21

9 years ago
Created attachment 334901 [details] [diff] [review]
Update trunk to libpng-1.2.31 (ignore whitespace changes)

Libpng-1.2.31 has been released.
(Assignee)

Comment 22

9 years ago
Created attachment 334902 [details] [diff] [review]
Update trunk to libpng-1.2.31
Attachment #320539 - Attachment is obsolete: true
Attachment #320539 - Flags: review?(tor)
I'll try this out tonight and let you know how it goes.
Summary: Update trunk to libpng-1.2.29 → Update trunk to libpng-1.2.31
   Creating library xul.lib and object xul.exp
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_handle_fdAT
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_handle_acTL
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_have_info
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_progressive_read_reset
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_read_reinit
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_handle_fcTL
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _png_handle_fcTL
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_read_reset
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _png_read_reset
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _png_ensure_sequence_number
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _png_ensure_sequence_number
xul.dll : fatal error LNK1120: 8 unresolved externals
(Assignee)

Comment 25

9 years ago
Created attachment 334993 [details] [diff] [review]
Update trunk to libpng-1.2.31 (v1)

Changes "png_" prefix to "MOZ_PNG_" for exported APNG functions.
Attachment #334902 - Attachment is obsolete: true
(Assignee)

Comment 26

9 years ago
Created attachment 335030 [details] [diff] [review]
Update trunk to libpng-1.2.31 (v2)

Prefix more exported APNG functions with PNG_APNG
Attachment #334993 - Attachment is obsolete: true
(Assignee)

Comment 27

9 years ago
(In reply to comment #26)

> Prefix more exported APNG functions with PNG_APNG

I meant to say, with MOZ_APNG
(Assignee)

Updated

9 years ago
Blocks: 446582
(Assignee)

Comment 28

9 years ago
(In reply to comment #24)
>    Creating library xul.lib and object xul.exp
> png.lib(pngpread.obj) : error LNK2001: unresolved external symbol
> _png_handle_fdAT
>

This is curious, because the patch does not change anything with regard to the exported APNG functions, and they are treated exactly the same way as the similar "png_handle_*" functions in libpng proper.  Do you not see the same problem when trying to build the unpatched library from the trunk?  The only thing I can think of is some possible conflict with an existing system library, and the v1/v2 patches may help with that.
Well, v1 and v2 were giving me fits until I came upon a small typo in them :-)
-#define png_have_info                   MOZ_APNG_have info
+#define png_have_info                   MOZ_APNG_have_info

Sadly, I'm still getting the same errors. And yes, the current libpng in the tree works fine.
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_handle_fdAT
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_handle_acTL
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_have_info
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_prog_read_reset
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_read_reinit
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_handle_fcTL
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_handle_fcTL
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_read_reset
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_read_reset
png.lib(pngpread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_ensure_seqno
png.lib(pngread.obj) : error LNK2001: unresolved external symbol _MOZ_APNG_ensure_seqno
xul.dll : fatal error LNK1120: 8 unresolved externals
(Assignee)

Comment 30

9 years ago
Created attachment 335150 [details] [diff] [review]
Update trunk to libpng-1.2.31 (v3)

Several APNG conditional blocks were mislocated in patches v1 and v2.
Attachment #335030 - Attachment is obsolete: true
v3 compiles OK and the basic APNG demos seem to work OK. Also, as far as I can tell, attachment 320493 [details] seems to be working OK (assuming the desired behavior is to have a red square with a Firefox icon flashing periodically). However, the image linked from bug 433047 shows a column of pixels on the right side of the image while animating, which AFAICT isn't supposed to happen.
(Assignee)

Comment 32

9 years ago
(In reply to comment #31)

> the image linked from bug 433047 shows a column of pixels on the right side of
> the image while animating, which AFAICT isn't supposed to happen.
> 

Doesn't that also happen with the unpatched libpng?
Correct, but I was under the impression from comment #2 in that bug that it would be fixed by the newer libpng. If that's not the case, we should probably at least remove the dependency on this bug.
(Assignee)

Comment 34

9 years ago
Two one-byte overruns have been discovered in libpng-1.2.31 and are fixed in the latest beta (libpng-1.2.32beta01).  Neither of these are in parts of the code used by mozilla/firefox so there is no need to wait for libpng-1.2.32 on this account.
Attachment #335150 - Flags: superreview?(tor)
Attachment #335150 - Flags: review?(pavlov)
Comment on attachment 335150 [details] [diff] [review]
Update trunk to libpng-1.2.31 (v3)

stuart/tor, can you rubberstamp this?
No longer blocks: 433047
Tryserver builds for those who may be interested:
https://build.mozilla.org/tryserver-builds/2008-09-07_06:40-mcsmurf@mcsmurf.de-1220794773/

Updated

9 years ago
Attachment #335150 - Flags: superreview?(tor) → superreview+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Still needs stuart's blessing.
Keywords: checkin-needed
Attachment #335150 - Flags: review?(joe)
Attachment #335150 - Flags: review?(pavlov)
Attachment #335150 - Flags: review?(joe)
Attachment #335150 - Flags: review+
Keywords: checkin-needed
Pushed in http://hg.mozilla.org/mozilla-central/rev/3bd94f1980a1
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Keywords: checkin-needed

Comment 39

9 years ago
>Versions 1.2.30 and 1.2.31 of libpng can crash when reading images with multiple zTXt chunks; it is likely that this vulnerability could lead to a remote compromise in the case of a libpng-based browser visiting a hostile web site. This vulnerability has been assigned ID CVE-2008-3964 and is fixed in version 1.2.32, released 18 September 2008.  

libpng-1.2.33 is out.
(Assignee)

Comment 40

9 years ago
We do not read the zTXt chunk and are immune to this vulnerability, whether we use the embedded libpng or the system libpng.
(Assignee)

Updated

8 years ago
Blocks: 495609
You need to log in before you can comment on or make changes to this bug.