Libpng-1.2.34 and earlier contain several instances of malloc an array of N elements for (i=0; i<N; i++) malloc element[i]; If the application runs out of memory before the Nth element has been allocated, libpng will longjmp to a cleanup process. This process will attempt to free() all of the elements, some of which might be undefined. A malevolent web site could deliberately force this behavior. The fix is to store NULL in all of the elements before performing the loop that malloc's them. I will upload a patch shortly to upgrade the trunk to a preview of libpng-1.2.35 which addresses the problem. There are at least 5 instances of this problem. The first is in png_read_png() which we do not use. The second is in decoding the pCAL chunk, which we also do not use. The other three are near the end of pngrtran.c, where 16-bit gamma tables are built. I believe we do encounter this vulnerability, even though we use png_strip_16() to reduce the input to 8 bits. Because of the order of function calls within pngrtran.c, this does not prevent the 16-bit gamma tables from being built, when the input PNG has 16-bit depth. The security flag can be cleared once libpng-1.2.35 is released.

Thanks Glenn

Libpng-1.2.35 is presently scheduled for release on February 19, 2009.

Although we're past code-freeze for 3.0.7 our release date will be well after that. We may want to re-spin with this fix, or at least keep this on the short-list to take in a re-spin.

Going to re-spin 3.0.7 for this :-(

Comment on attachment 362738 [details] [diff] [review] Update trunk to libpng-1.2.35 sr=dveditz fwiw, but I'm not an imglib peer. Approving for the 1.9.0 branch

Comment on attachment 362738 [details] [diff] [review] Update trunk to libpng-1.2.35 r=joe

Reed wondered if our own extra APNG stuff would need a similar fix but from what I can see none of the APNG code contains the pattern in question. Would be better if someone who knows the code double-checks though (maybe Glenn already did that? I'm not sure if APNG is in the upstream or not). I wrote Glenn for clarification, and comment 3 means "don't check-in until Feb 19". He's willing to go with 00:00 UTC but that would still mean builds don't start until Thursday morning because I'm sure Ben doesn't want to start a long chunk of work after 7pm his time.

(In reply to comment #8) > I wrote Glenn for clarification, and comment 3 means "don't check-in until Feb > 19". He's willing to go with 00:00 UTC but that would still mean builds don't > start until Thursday morning because I'm sure Ben doesn't want to start a long > chunk of work after 7pm his time. If that's the case, then let's get it checked in as late as possible on Wednesday night. If Ben is planning on starting builds sooner, we can check it in sooner. We can work this out tomorrow, in any case.

(In reply to comment #8) > Reed wondered if our own extra APNG stuff would need a similar fix but from > what I can see none of the APNG code contains the pattern in question. Would be > better if someone who knows the code double-checks though (maybe Glenn already > did that? I'm not sure if APNG is in the upstream or not). The APNG stuff looks OK with respect to this bug. No, APNG isn't in the libpng upstream, but I do keep an eye on it anyway.

Libpng-1.2.35 is out. Please clear the security flag.

Checked into CVS head (1.9.0) and GECKO190_20090217_RELBRANCH (1.9.0.7). The 1.9.0 branch was on 1.2.24 and after talking to Pav we figured it was safer to just take the mozilla-central version (1.2.34) and patch that.

Martin: What version of libpng is 1.8.1 on? It's probably worth upgrading it all the way to 1.2.35.

(In reply to comment #14) > Martin: What version of libpng is 1.8.1 on? 1.8.1.21pre (what you get when checking out 1.8) is at libpng-1.2.7 dated September 12, 2004. There have been a few security bugs reported since then and it appears that maybe one of them has been fixed in our embedded copy. The minimal patch above only addresses the most recent vulnerability. I think some of the others are just as bad.

(In reply to comment #15) > I think some of the others are just as bad. Or maybe not. The last one had to do with using lcms which the 1.8 branch does not use. The main reason for upgrading would be to not have to look back over 5 years of changelogs and bug reports to figure out if we are safe or not. #:-(

I'm all for doing a full update on 1.8.1. Glenn, would you be willing to work a patch up for that?

(In reply to comment #17) > I'm all for doing a full update on 1.8.1. Glenn, would you be willing to work a > patch up for that? OK. It'll be huge because part of change since 1.2.7 is the removal of the unlicensed Intel assembler code (pngvcrd.c and pnggcrd.c). Also there are many cosmetic changes (indentation, whitespace).

Comment on attachment 363218 [details] [diff] [review] Complete 1.8.1 patch, part 1 Joe: Wanna take a gander at this?

Comment on attachment 363218 [details] [diff] [review] Complete 1.8.1 patch, part 1 Thanks Joe. Martin, feel free to land this on 1.8.1 when you get chance. Approved for 1.8.1.21. a=ss

Thanks guys.

I inadvertently zeroed out libimg/png/mozpngconf.h in the big 2-part patch. Libpng should run OK without it but will be about 50-100kbytes larger than necessary. Some minor revision of Makefile.in in libimg/png and in libpr0n/decoders/png may be required to make mozpngconf.h work properly. I can provide a patch to restore this functionality later on this afternoon.

Apply this patch after applying the big 2-part patch, to restore mozpngconf.h and revise libpr0n/decoders/png/Makefile.in and libimg/png/Makefile.in

I could have removed the "APNG additions" block from mozpngconf.h, but it doesn't hurt anything to leave it in.

Comment on attachment 363219 [details] [diff] [review] Complete 1.8.1 patch, part 2 sr=dveditz on all three 1.8.1 patches.

Checked into the 1.8 branch.

This seems to cause failure of building Camino 1.6-M1.8. See <http://tinderbox.mozilla.org/showbuilds.cgi?tree=Camino>.

The only problem I know of with libpng-1.2.35 is in the png_debug() definition. Is Camino 1.6-M1.8 being built with debug on?

(In reply to comment #31) > The only problem I know of with libpng-1.2.35 is in the png_debug() definition. > Is Camino 1.6-M1.8 being built with debug on? Maybe with debuf off. I'm not a developer and I only build my own build with "--disable-debug" option, but I get build error same as http://tinderbox.mozilla.org/showlog.cgi?log=Camino/1235822040.1235822488.6445.gz _______ make[6]: *** [nsPNGDecoder.o] Error 1 make[5]: *** [libs] Error 2 make[4]: *** [libs] Error 2 make[3]: *** [tier_9] Error 2 make[2]: *** [alldep] Error 2 make[1]: *** [alldep] Error 2 make: *** [alldep] Error 2

(In reply to comment #32) > (In reply to comment #31) > but I get build error same as > http://tinderbox.mozilla.org/showlog.cgi?log=Camino/1235822040.1235822488.6445.gz > _______ That error log appears to be saying that it is using libpr0n/decoders/png with the "system libpng". Are you in fact using the system libpng and not the embedded one in libimg/png ?

When using the "system libpng" you have to include the system png.h and not the embedded png.h in libimg/png. It appears that there might be a mixup in this regard in the camino build script and the '#include "png.h"' directive inlibpr0n/decoders/png/nsPNGDecoder.cpp .

Please try this with your camino build: In nsPNGDecoder.cpp, change #include "png.h" to #ifdef MOZ_PNG_READ # include "png.h" #else # include <png.h> #endif

(In reply to comment #35) > Please try this with your camino build: > # include <png.h> If this works I will prepare a patch. dveditz: Would you prefer a new small patch to the current 1.8.1 branch (with all three branch patches already applied) or a complete replacement of the "restore mozpngconf.h" patch?

(In reply to comment #35) > Please try this with your camino build: > > In nsPNGDecoder.cpp, change > > #include "png.h" > > to > > #ifdef MOZ_PNG_READ > # include "png.h" > #else > # include <png.h> > #endif I test my Camino build with nsPNGDecoder.cpp changed as mentioned above, but I got a same error; ________ error: 'MOZ_PNG_get_progressive_ptr' was not declared in this scope make[5]: *** [nsPNGDecoder.o] Error 1 make[4]: *** [libs] Error 2 make[3]: *** [libs] Error 2 make[2]: *** [tier_9] Error 2 make[1]: *** [alldep] Error 2 make: *** [alldep] Error 2

(In reply to comment #37) > (In reply to comment #35) > build with nsPNGDecoder.cpp changed as mentioned above, but I > got a same error; Please try the same except test MOZ_NATIVE_PNG. I believe it is 0 when using the embedded library and 1 when using the system library. It's a copy of $SYSTEM_PNG from configure.in. #if (MOZ_NATIVE_PNG == 0) # include "png.h" #else # include <png.h> #endif

On Sat, Feb 28, 2009 at 9:54 AM, I wrote: > #if (MOZ_NATIVE_PNG == 0) > # include "png.h" > #else > # include <png.h> > #endif Maybe it would be safer to do it the other way around, to avoid any confusion between "0", "'0'", " ", and "": #if (MOZ_NATIVE_PNG == 1) # include <png.h> #else # include "png.h" #endif

(In reply to comment #38) > Please try the same except test MOZ_NATIVE_PNG. I believe it is > 0 when using the embedded library and 1 when using the system library. > It's a copy of $SYSTEM_PNG from configure.in. > > #if (MOZ_NATIVE_PNG == 0) > # include "png.h" > #else > # include <png.h> > #endif That's the same. ________________ error: 'MOZ_PNG_get_progressive_ptr' was not declared in this scope make[5]: *** [nsPNGDecoder.o] Error 1 make[4]: *** [libs] Error 2 make[3]: *** [libs] Error 2 make[2]: *** [tier_9] Error 2 make[1]: *** [alldep] Error 2 make: *** [alldep] Error 2 Just for reference, I'm building with .mozconfig ________________ . $topsrcdir/camino/config/mozconfig mk_add_options MOZ_MAKE_FLAGS=-j4 mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@ ac_add_options --enable-reorder ac_add_options --enable-strip ac_add_options --enable-static ac_add_options --enable-static-libs ac_add_options --enable-pthreads ac_add_options --enable-optimize="-O3 -march=nocona -fforce-addr -msse3 -mfpmath=sse" ac_add_options --disable-debug ac_add_options --disable-shared

(In reply to comment #39) > Maybe it would be safer to do it the other way around, to avoid > any confusion between "0", "'0'", " ", and "": > > #if (MOZ_NATIVE_PNG == 1) > # include <png.h> > #else > # include "png.h" > #endif Sorry, failure to build and I get the same error.

What happens when you put ac_add_options --without-system-png in your .mozconfig?

I just checked out Mozilla-1.8 and find that libimg/png/mozpngconf.h has been updated according to the "Restore mozpngconf.h" patch but that libpr0n/decoders/png/Makefile.in was not. That would probably explain the camino behavior that was reported.

Not just Camino. Dan checked this in on 1.8 and everything is currently red. http://tinderbox.mozilla.org/showbuilds.cgi?tree=Mozilla1.8 Dan, please either back this out or land a fix.

(In reply to comment #43) > I just checked out Mozilla-1.8 and find that libimg/png/mozpngconf.h has been > updated according to the "Restore mozpngconf.h" patch but that > libpr0n/decoders/png/Makefile.in was not. That would probably explain the > camino behavior that was reported. (In reply to comment #44) > Dan, please either back this out or land a fix. I just checked in the missing hunk that Glenn identified in comment 43; I hope that was all that was missing.

(In reply to comment #45) > I just checked in the missing hunk that Glenn identified in comment 43; I hope > that was all that was missing. I succeeded in building my Camino.

(In reply to comment #45) > I just checked in the missing hunk that Glenn identified in comment 43; I hope > that was all that was missing. Thanks Smokey. It looks like the Camino 1.8 tree is building fine now as well. Hopefully Firefox and Thunderbird recover soon as well. (fwiw, I gave Smokey approval to do this on IRC.)

SeaMonkey Linux hoshi, the last box that was red from the 1.8 checkin here, went green about a half hour ago; the Firefox and Thunderbird boxes on Mozilla1.8 went green earlier this afternoon, as did Camino's 1.8 box. Looks like Glenn reopened this after Eiichi's comments, so setting back to FIXED now that all the 1.8 boxen are green.

It was not FIXED before, it had landed only on the old branches (represented by keywords, not bug status). It is fixed now, however http://hg.mozilla.org/mozilla-central/rev/ad8d75516c5e http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b3b4d684496d

Verified code is checked in for 1.9.0.

Verified by checkin on trunk and 1.9.1 too.

Mass change: adding fixed1.9.2 keyword (This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)