Bug 1526717. Guard against libpng calling the info callback more than once. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 14 Mar 2019 14:32:37 -0500
changeset 464088 355c9ff9b89581cf5b2ff03f3df9df05939f89c3
parent 464030 25f9343a6480f39a240f1862b588db7dba99efde
child 464089 65a5625c479045150d7990b93a64b9a5e00ffb82
push id35708
push userrmaries@mozilla.com
push dateFri, 15 Mar 2019 03:45:36 +0000
treeherdermozilla-central@c525a24dffc3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1526717
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1526717. Guard against libpng calling the info callback more than once. r=aosmond libpng uses the first IDAT chunk it encounters as a signal that it has read all header chunks and to send the info callback. The testcase png has an IDAT chunk, then a z chunk (not a known chunk type), and then another IDAT chunk. libpng tracks if we are in an "after idat" state, and throws a benign error if it encounters another IDAT chunk in "after idat" mode, but it just continues normally, processing the idat chunk as if it were the first and therefore sends the info callback again. This seems silly. https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/media/libpng/pngpread.c#307
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
image/test/crashtests/1526717-1.html
image/test/crashtests/1526717-1.png
image/test/crashtests/crashtests.list
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -112,16 +112,17 @@ nsPNGDecoder::nsPNGDecoder(RasterImage* 
       mInProfile(nullptr),
       mTransform(nullptr),
       mFormat(SurfaceFormat::UNKNOWN),
       mCMSMode(0),
       mChannels(0),
       mPass(0),
       mFrameIsHidden(false),
       mDisablePremultipliedAlpha(false),
+      mGotInfoCallback(false),
       mNumFrames(0) {}
 
 nsPNGDecoder::~nsPNGDecoder() {
   if (mPNG) {
     png_destroy_read_struct(&mPNG, mInfo ? &mInfo : nullptr, nullptr);
   }
   if (mCMSLine) {
     free(mCMSLine);
@@ -532,16 +533,23 @@ void nsPNGDecoder::info_callback(png_str
   unsigned int channels;
 
   png_bytep trans = nullptr;
   int num_trans = 0;
 
   nsPNGDecoder* decoder =
       static_cast<nsPNGDecoder*>(png_get_progressive_ptr(png_ptr));
 
+  if (decoder->mGotInfoCallback) {
+    MOZ_LOG(sPNGLog, LogLevel::Warning, ("libpng called info_callback more than once\n"));
+    return;
+  }
+
+  decoder->mGotInfoCallback = true;
+
   // Always decode to 24-bit RGB or 32-bit RGBA
   png_get_IHDR(png_ptr, info_ptr, &width, &height, &bit_depth, &color_type,
                &interlace_type, &compression_type, &filter_type);
 
   const IntRect frameRect(0, 0, width, height);
 
   // Post our size to the superclass
   decoder->PostSize(frameRect.Width(), frameRect.Height());
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -98,16 +98,17 @@ class nsPNGDecoder : public Decoder {
 
   // whether CMS or premultiplied alpha are forced off
   uint32_t mCMSMode;
 
   uint8_t mChannels;
   uint8_t mPass;
   bool mFrameIsHidden;
   bool mDisablePremultipliedAlpha;
+  bool mGotInfoCallback;
 
   struct AnimFrameInfo {
     AnimFrameInfo();
 #ifdef PNG_APNG_SUPPORTED
     AnimFrameInfo(png_structp aPNG, png_infop aInfo);
 #endif
 
     DisposalMethod mDispose;
new file mode 100644
--- /dev/null
+++ b/image/test/crashtests/1526717-1.html
@@ -0,0 +1,1 @@
+<img height="64" width="64" src="fuzz-1311.png?0.5592939664601271">
\ No newline at end of file
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..5ac5b32744c0da609e7c5cd99a7a25ff62e951aa
GIT binary patch
literal 318
zc%17D@N?(olHy`uVBq!ia0vp^CxCc02NMGW<L%gz)j*DRiEBj3e}S#-o-U3d6}Qgp
z<=oSWsZ9ZdS%3t?<cBvjffP%+qpu?aW7`757t`W_e5R5hzhDM-!xvr+KptmVfnz$5
zHUwfFhL~RAS3pJ<$cBi+Z?7Bj9Z=wL4h;Spck<%o*`*9=n-l`R?%j}5@hRz|lGF0f
zbJ#pC-Ih0wo|m>vr93|FTiNUB;U<hsEF1y~t1{QTI1zDC_I5=4GwoRnV8N>!78y(b
yU$jO8DDUt`<Y3pinwk`q=&C2>8-TVdSApD9;1OBOz`%D1gc(IOP#md)!;t{?O<>~y
--- a/image/test/crashtests/crashtests.list
+++ b/image/test/crashtests/crashtests.list
@@ -53,8 +53,10 @@ load invalid-size.gif
 load invalid-size-second-frame.gif
 
 load multiple-png-hassize.ico # Bug 863958 - This icon's size is such that it leads to multiple writes to the PNG decoder after we've gotten our size.
 asserts(0-2) load ownerdiscard.html # Bug 1323672, bug 807211
 load truncated-second-frame.png # Bug 863975
 
 # Bug 1509998 - Ensure that we handle empty frame rects in animated images.
 load 1509998.gif
+
+load 1526717-1.html