Bug 1434490. Avoid overflow in nsPNGEncoder::WriteCallback. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Fri, 28 Sep 2018 23:23:01 -0500
changeset 494582 0236f549f73f1e220ba0d44e7a3e350c252dd2a4
parent 494581 5474778a6ee33a20620ba2a2b962456887c082bf
child 494583 d45a7a0f6f297eb90e475f36158508f605c021d4
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1434490
milestone64.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 1434490. Avoid overflow in nsPNGEncoder::WriteCallback. r=aosmond
image/encoders/png/nsPNGEncoder.cpp
image/encoders/png/nsPNGEncoder.h
--- a/image/encoders/png/nsPNGEncoder.cpp
+++ b/image/encoders/png/nsPNGEncoder.cpp
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ImageLogging.h"
 #include "nsCRT.h"
 #include "nsPNGEncoder.h"
 #include "nsStreamUtils.h"
 #include "nsString.h"
 #include "prprf.h"
+#include "mozilla/CheckedInt.h"
 
 using namespace mozilla;
 
 static LazyLogModule sPNGEncoderLog("PNGEncoder");
 
 NS_IMPL_ISUPPORTS(nsPNGEncoder, imgIEncoder, nsIInputStream,
                   nsIAsyncInputStream)
 
@@ -698,40 +699,65 @@ void // static
 nsPNGEncoder::WriteCallback(png_structp png, png_bytep data,
                             png_size_t size)
 {
   nsPNGEncoder* that = static_cast<nsPNGEncoder*>(png_get_io_ptr(png));
   if (!that->mImageBuffer) {
     return;
   }
 
-  if (that->mImageBufferUsed + size > that->mImageBufferSize) {
+  CheckedUint32 sizeNeeded = CheckedUint32(that->mImageBufferUsed) + size;
+  if (!sizeNeeded.isValid()) {
+    // Take the lock to ensure that nobody is trying to read from the buffer
+    // we are destroying
+    ReentrantMonitorAutoEnter autoEnter(that->mReentrantMonitor);
+
+    that->NullOutImageBuffer();
+    return;
+  }
+
+  if (sizeNeeded.value() > that->mImageBufferSize) {
     // When we're reallocing the buffer we need to take the lock to ensure
     // that nobody is trying to read from the buffer we are destroying
     ReentrantMonitorAutoEnter autoEnter(that->mReentrantMonitor);
 
-    // expand buffer, just double each time
-    that->mImageBufferSize *= 2;
-    uint8_t* newBuf = (uint8_t*)realloc(that->mImageBuffer,
-                                        that->mImageBufferSize);
-    if (!newBuf) {
-      // can't resize, just zero (this will keep us from writing more)
-      free(that->mImageBuffer);
-      that->mImageBuffer = nullptr;
-      that->mImageBufferSize = 0;
-      that->mImageBufferUsed = 0;
-      return;
+    while (sizeNeeded.value() > that->mImageBufferSize) {
+      // expand buffer, just double each time
+      CheckedUint32 bufferSize = CheckedUint32(that->mImageBufferSize) * 2;
+      if (!bufferSize.isValid()) {
+        that->NullOutImageBuffer();
+        return;
+      }
+      that->mImageBufferSize *= 2;
+      uint8_t* newBuf = (uint8_t*)realloc(that->mImageBuffer,
+                                          that->mImageBufferSize);
+      if (!newBuf) {
+        // can't resize, just zero (this will keep us from writing more)
+        that->NullOutImageBuffer();
+        return;
+      }
+      that->mImageBuffer = newBuf;
     }
-    that->mImageBuffer = newBuf;
   }
+
   memcpy(&that->mImageBuffer[that->mImageBufferUsed], data, size);
   that->mImageBufferUsed += size;
   that->NotifyListener();
 }
 
+void nsPNGEncoder::NullOutImageBuffer()
+{
+  mReentrantMonitor.AssertCurrentThreadIn();
+
+  free(mImageBuffer);
+  mImageBuffer = nullptr;
+  mImageBufferSize = 0;
+  mImageBufferUsed = 0;
+}
+
 void
 nsPNGEncoder::NotifyListener()
 {
   // We might call this function on multiple threads (any threads that call
   // AsyncWait and any that do encoding) so we lock to avoid notifying the
   // listener twice about the same data (which generally leads to a truncated
   // image).
   ReentrantMonitorAutoEnter autoEnter(mReentrantMonitor);
--- a/image/encoders/png/nsPNGEncoder.h
+++ b/image/encoders/png/nsPNGEncoder.h
@@ -49,16 +49,17 @@ protected:
                         uint32_t* offsetY);
   void ConvertHostARGBRow(const uint8_t* aSrc, uint8_t* aDest,
                           uint32_t aPixelWidth, bool aUseTransparency);
   void StripAlpha(const uint8_t* aSrc, uint8_t* aDest,
                   uint32_t aPixelWidth);
   static void WarningCallback(png_structp png_ptr, png_const_charp warning_msg);
   static void ErrorCallback(png_structp png_ptr, png_const_charp error_msg);
   static void WriteCallback(png_structp png, png_bytep data, png_size_t size);
+  void NullOutImageBuffer();
   void NotifyListener();
 
   png_struct* mPNG;
   png_info* mPNGinfo;
 
   bool mIsAnimation;
   bool mFinished;