Bug 695421 - Fixed crash for ICO files that have a large (and wrong) width specified. r=joe
authorBrian R. Bondy <netzen@gmail.com>
Fri, 04 Nov 2011 09:56:17 -0400
changeset 81408 6a4f90ea47ae91407446003b3a1458ef33945665
parent 81407 5bbc4fb5b277ea0212f4c210f2a7238dbbe70904
child 81409 dd25b9224c768f86fa8bd7d988466f146b42d9d6
push id90
push userffxbld
push dateSun, 29 Jan 2012 07:46:52 +0000
treeherdermozilla-release@acddb6b6a01c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe
bugs695421
milestone10.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 695421 - Fixed crash for ICO files that have a large (and wrong) width specified. r=joe
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -161,22 +161,67 @@ bool nsICODecoder::FillBitmapFileHeaderB
   dataOffset = NATIVE32_TO_LITTLE(dataOffset);
   memcpy(bfh + 10, &dataOffset, sizeof(dataOffset));
   return true;
 }
 
 // A BMP inside of an ICO has *2 height because of the AND mask
 // that follows the actual bitmap.  The BMP shouldn't know about
 // this difference though.
-void 
-nsICODecoder::FillBitmapInformationBufferHeight(PRInt8 *bih) 
+bool
+nsICODecoder::FixBitmapHeight(PRInt8 *bih) 
 {
-  PRInt32 height = GetRealHeight();
+  // Get the height from the BMP file information header
+  PRInt32 height;
+  memcpy(&height, bih + 8, sizeof(height));
+  height = LITTLE_TO_NATIVE32(height);
+
+  // The bitmap height is by definition * 2 what it should be to account for
+  // the 'AND mask'. It is * 2 even if the `AND mask` is not present.
+  height /= 2;
+
+  if (height > 256) {
+    return false;
+  }
+
+  // We should always trust the height from the bitmap itself instead of 
+  // the ICO height.  So fix the ICO height.
+  if (height == 256) {
+    mDirEntry.mHeight = 0;
+  } else {
+    mDirEntry.mHeight = (PRInt8)height;
+  }
+
+  // Fix the BMP height in the BIH so that the BMP decoder can work properly
   height = NATIVE32_TO_LITTLE(height);
   memcpy(bih + 8, &height, sizeof(height));
+  return true;
+}
+
+// We should always trust the contained resource for the width
+// information over our own information.
+bool
+nsICODecoder::FixBitmapWidth(PRInt8 *bih) 
+{
+  // Get the width from the BMP file information header
+  PRInt32 width;
+  memcpy(&width, bih + 4, sizeof(width));
+  width = LITTLE_TO_NATIVE32(width);
+  if (width > 256) {
+    return false;
+  }
+
+  // We should always trust the width  from the bitmap itself instead of 
+  // the ICO width.
+  if (width == 256) {
+    mDirEntry.mWidth = 0;
+  } else {
+    mDirEntry.mWidth = (PRInt8)width;
+  }
+  return true;
 }
 
 // The BMP information header's bits per pixel should be trusted
 // more than what we have.  Usually the ICO's BPP is set to 0
 PRInt32 
 nsICODecoder::ExtractBPPFromBitmap(PRInt8 *bih)
 {
   PRInt32 bitsPerPixel;
@@ -405,18 +450,28 @@ nsICODecoder::WriteInternal(const char* 
     mDataError = mContainedDecoder->HasDataError();
     if (mContainedDecoder->HasDataError()) {
       return;
     }
 
     // Setup the cursor hot spot if one is present
     SetHotSpotIfCursor();
 
-    // Fix the height on the BMP resource
-    FillBitmapInformationBufferHeight((PRInt8*)mBIHraw);
+    // Fix the ICO height from the BIH.
+    // Fix the height on the BIH to be /2 so our BMP decoder will understand.
+    if (!FixBitmapHeight(reinterpret_cast<PRInt8*>(mBIHraw))) {
+      PostDataError();
+      return;
+    }
+
+    // Fix the ICO width from the BIH.
+    if (!FixBitmapWidth(reinterpret_cast<PRInt8*>(mBIHraw))) {
+      PostDataError();
+      return;
+    }
 
     // Write out the BMP's bitmap info header
     mContainedDecoder->Write(mBIHraw, sizeof(mBIHraw));
     mDataError = mContainedDecoder->HasDataError();
     if (mContainedDecoder->HasDataError()) {
       return;
     }
 
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -78,18 +78,24 @@ public:
 
 private:
   // Processes a single dir entry of the icon resource
   void ProcessDirEntry(IconDirEntry& aTarget);
   // Sets the hotspot property of if we have a cursor
   void SetHotSpotIfCursor();
   // Creates a bitmap file header buffer, returns true if successful
   bool FillBitmapFileHeaderBuffer(PRInt8 *bfh);
-  // Fixes the height of a BMP information header field
-  void FillBitmapInformationBufferHeight(PRInt8 *bih);
+  // Fixes the ICO height to match that of the BIH.
+  // and also fixes the BIH height to be /2 of what it was.
+  // See definition for explanation.
+  // Returns false if invalid information is contained within.
+  bool FixBitmapHeight(PRInt8 *bih);
+  // Fixes the ICO width to match that of the BIH.
+  // Returns false if invalid information is contained within.
+  bool FixBitmapWidth(PRInt8 *bih);
   // Extract bitmap info header size count from BMP information header
   PRInt32 ExtractBIHSizeFromBitmap(PRInt8 *bih);
   // Extract bit count from BMP information header
   PRInt32 ExtractBPPFromBitmap(PRInt8 *bih);
   // Calculates the row size in bytes for the AND mask table
   PRUint32 CalcAlphaRowSize();
   // Obtains the number of colors from the BPP, mBPP must be filled in
   PRUint16 GetNumColors();