Last Comment Bug 734317 - libpng: Confirmation of another heap-buffer overflow not affecting mozilla
: libpng: Confirmation of another heap-buffer overflow not affecting mozilla
Status: RESOLVED FIXED
[advisory-tracking+][sg:nse][qa-] aff...
:
Product: Firefox
Classification: Client Software
Component: Untriaged (show other bugs)
: 11 Branch
: x86_64 Linux
: -- critical (vote)
: ---
Assigned To: Joe Drew (not getting mail)
:
:
Mentors:
Depends on: 648690
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-08 21:15 PST by Huzaifa Sidhpurwala
Modified: 2012-07-11 20:00 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
unaffected


Attachments

Description Huzaifa Sidhpurwala 2012-03-08 21:15:27 PST
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.
Comment 1 Al Billings [:abillings] 2012-03-09 10:01:01 PST
Adding some folks from the other libpng issue to make sure this is seen and commented upon.
Comment 2 Huzaifa Sidhpurwala 2012-03-10 06:02:49 PST
Anything on this yet?
Comment 3 Daniel Veditz [:dveditz] 2012-03-12 01:56:52 PDT
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".
Comment 4 Huzaifa Sidhpurwala 2012-03-12 03:12:42 PDT
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.
Comment 5 Glenn Randers-Pehrson 2012-03-12 04:30:52 PDT
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).
Comment 6 Daniel Veditz [:dveditz] 2012-03-14 16:42:33 PDT
We all seem to agree Mozilla code is not affected.
Comment 7 Glenn Randers-Pehrson 2012-04-13 11:21:49 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.