Closed Bug 418900 Opened 14 years ago Closed 13 years ago

Update trunk to libpng-1.2.31

Categories

(Core :: ImageLib, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: glennrp+bmo, Assigned: glennrp+bmo)

References

(Blocks 1 open bug, )

Details

Attachments

(3 files, 12 obsolete files)

22.45 KB, image/png
Details
209.24 KB, patch
Details | Diff | Splinter Review
353.04 KB, patch
joe
: review+
Details | Diff | Splinter Review
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.
Attached patch Update trunk to libpng-1.2.25 (obsolete) — Splinter Review
Attachment #304823 - Flags: review?(tor)
FYI, the attached patch appears to have Windows line endings.
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.
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.
Attached patch update trunk to libpng-1.2.26 (obsolete) — Splinter Review
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
Attachment #313392 - Flags: review?(tor)
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)
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.
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
Attachment #314396 - Attachment is obsolete: true
Attachment #314396 - Flags: review?(pavlov)
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.
No longer blocks: 428045
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
Here is a preview of libpng-1.2.27 which will be released around April 30.
Attached patch Update trunk to libpng-1.2.27 (obsolete) — Splinter Review
Libpng-1.2.27 has been released.
Attachment #316078 - Attachment is obsolete: true
Attachment #318302 - Flags: review?(pavlov)
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.
Attached patch Update trunk to libpng-1.2.29 (obsolete) — Splinter Review
Libpng-1.2.29 has been released.
Attachment #318302 - Attachment is obsolete: true
Attachment #320262 - Flags: review?(tor)
Attachment #318302 - Flags: review?(pavlov)
Summary: Update trunk to libpng-1.2.27 → Update trunk to libpng-1.2.29
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.
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.
Revised patch per Andrew's comment.
Attachment #320262 - Attachment is obsolete: true
Attachment #320539 - Flags: review?(tor)
Attachment #320262 - Flags: review?(tor)
Sadly, it looks like with that the image in question works but several more others no longer do.
(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)?

Blocks: 433047
Libpng-1.2.31 has been released.
Attached patch Update trunk to libpng-1.2.31 (obsolete) — Splinter Review
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
Changes "png_" prefix to "MOZ_PNG_" for exported APNG functions.
Attachment #334902 - Attachment is obsolete: true
Prefix more exported APNG functions with PNG_APNG
Attachment #334993 - Attachment is obsolete: true
(In reply to comment #26)

> Prefix more exported APNG functions with PNG_APNG

I meant to say, with MOZ_APNG
Blocks: 446582
(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
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.
(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.
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
Attachment #335150 - Flags: superreview?(tor) → superreview+
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+
Pushed in http://hg.mozilla.org/mozilla-central/rev/3bd94f1980a1
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
>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.
We do not read the zTXt chunk and are immune to this vulnerability, whether we use the embedded libpng or the system libpng.
Blocks: 495609
You need to log in before you can comment on or make changes to this bug.