libpng: Confirmation of another heap-buffer overflow not affecting mozilla

RESOLVED FIXED

Status

()

Firefox
Untriaged
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Huzaifa Sidhpurwala, Assigned: Joe Drew (not getting mail))

Tracking

11 Branch
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox13 fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [advisory-tracking+][sg:nse][qa-] affects non-mozilla products; ok to clear the security flag now)

(Reporter)

Description

6 years ago
I am not sure if glenn already informed mozilla about this issue. (Most probably he already did and i am being paranoid).

The following was fixed in libpng and in chrome as well:
http://src.chromium.org/viewvc/chrome?view=rev&revision=125311

This is similar to the previous libpng which i had reported. Glenn provided the following explanation of why moz. would not have been affected by this:

"Chunks that you know you will never use should be listed in a "png_set_keep_unknown_chunks()" call with the property PNG_HANDLE_CHUNK_NEVER as demonstrated in libpng's contrib/gregbook/readpng2.c, which will prevent the listed
chunks from being processed.  Mozilla uses this method to
skip zTXt, iTXt, and other chunks that it doesn't use, and
has consequently not been affected by most of the CVEs involving
libpng in recent years.

You can prevent DoS by large zTXt, etc., chunks by setting
limits on the amount of expansion allowed.  Mozilla, for
example, sets a limit of 4MB on the maximum size of an iCCP
chunk after decompression."

Looking at the code, i seem to agree with the above. But since i am not an expert can someone with more knowledge please comment?

Thanks.
Adding some folks from the other libpng issue to make sure this is seen and commented upon.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

6 years ago
Anything on this yet?
We have this fix on trunk already, as part of the upgrade to libpng 1.5.9 in bug 648690: http://hg.mozilla.org/mozilla-central/annotate/5ec9524de1af/media/libpng/pngrutil.c#l343

Even in unfixed code we skip over the bug the first time png_inflate() is called from png_decrompress_chunk() because 0 is passed as output and output_size, and we'll only fall into this code on the second call to png_inflate() if the expanded size is < 4Mb as noted in comment 0.

We should be able to close this one "worksforme" or "invalid".
Assignee: nobody → joe
status-firefox13: --- → fixed
Whiteboard: fixed by 648690
(Reporter)

Comment 4

6 years ago
Daniel,

The Update to 1.5.9 is not applicable to ESR, that would mean ESR is still vulnerable, if you can call this as a vulnerability as such.
We have been using the png_set_keep_chunk() mechanism to ignore zTXt and iTXt chunks for quite a long time.  The only compressed chunks we process are IDAT (which is decompressed by different code) and iCCP (which only calls png_inflate() once).
We all seem to agree Mozilla code is not affected.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Depends on: 648690
Resolution: --- → FIXED
Whiteboard: fixed by 648690 → [sg:nse] affects non-mozilla products
status-firefox-esr10: --- → unaffected
The security flag on this bug can be cleared now.  There is nothing here that isn't exposed in public CVEs and in the source for libpng-1.5.10.
Whiteboard: [sg:nse] affects non-mozilla products → [sg:nse] affects non-mozilla products; ok to clear the security flag now
Whiteboard: [sg:nse] affects non-mozilla products; ok to clear the security flag now → [sg:nse][qa-] affects non-mozilla products; ok to clear the security flag now
Whiteboard: [sg:nse][qa-] affects non-mozilla products; ok to clear the security flag now → [advisory-tracking+][sg:nse][qa-] affects non-mozilla products; ok to clear the security flag now
Group: core-security
You need to log in before you can comment on or make changes to this bug.