Bug 789046 - Port a bug fix from the Webkit GIF decoder back to its progenitor, Mozilla! r=jlebar
authorJoe Drew <joe@drew.ca>
Fri, 21 Sep 2012 18:32:49 -0400
changeset 114641 e776f05c4f80441039eb04104899579fc6660a5f
parent 114640 e36ba60ece12c789f2fae22546a399543afa3c02
child 114642 dccfebdf756cd85a4ee416e9f2bf569b28afcb82
push id239
push userakeybl@mozilla.com
push dateThu, 03 Jan 2013 21:54:43 +0000
treeherdermozilla-release@3a7b66445659 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlebar
bugs789046
milestone18.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 789046 - Port a bug fix from the Webkit GIF decoder back to its progenitor, Mozilla! r=jlebar
image/decoders/nsGIFDecoder2.cpp
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -768,31 +768,55 @@ nsGIFDecoder2::WriteInternal(const char 
           } else {
             /* No images decoded, there is nothing to display. */
             mGIFStruct.state = gif_error;
           }
       }
       break;
 
     case gif_extension:
+      // Comment taken directly from WebKit's GIFImageReader.cpp.
+      //
+      // The GIF spec mandates lengths for three of the extensions below.
+      // However, it's possible for GIFs in the wild to deviate. For example,
+      // some GIFs that embed ICC color profiles using gif_application_extension
+      // violate the spec and treat this extension block like a sort of
+      // "extension + data" block, giving a size greater than 11 and filling the
+      // remaining bytes with data (then following with more data blocks as
+      // needed), instead of placing a true data block just after the 11 byte
+      // extension block.
+      //
+      // Accordingly, if the specified length is larger than the required value,
+      // we use it. If it's smaller, then we enforce the spec value, because the
+      // parsers for these extensions expect to have the specified number of
+      // bytes available, and if we don't ensure that, they could read off the
+      // end of the heap buffer. (In this case, it's likely the GIF is corrupt
+      // and we'll soon fail to decode anyway.)
       mGIFStruct.bytes_to_consume = q[1];
       if (mGIFStruct.bytes_to_consume) {
         switch (*q) {
         case GIF_GRAPHIC_CONTROL_LABEL:
           mGIFStruct.state = gif_control_extension;
+          mGIFStruct.bytes_to_consume = NS_MAX(mGIFStruct.bytes_to_consume, 4u);
           break;
-  
+
         case GIF_APPLICATION_EXTENSION_LABEL:
           mGIFStruct.state = gif_application_extension;
+          mGIFStruct.bytes_to_consume = NS_MAX(mGIFStruct.bytes_to_consume, 11u);
           break;
-  
+
+        case GIF_PLAIN_TEXT_LABEL:
+          mGIFStruct.state = gif_skip_block;
+          mGIFStruct.bytes_to_consume = NS_MAX(mGIFStruct.bytes_to_consume, 12u);
+          break;
+
         case GIF_COMMENT_LABEL:
           mGIFStruct.state = gif_consume_comment;
           break;
-  
+
         default:
           mGIFStruct.state = gif_skip_block;
         }
       } else {
         GETN(1, gif_image_start);
       }
       break;