Bug 789046 - Port a bug fix from the Webkit GIF decoder back to its progenitor, Mozilla! r=jlebar a=lsblakk
authorJoe Drew <joe@drew.ca>
Fri, 21 Sep 2012 18:32:49 -0400
changeset 109618 aa46c50b774730cc1d3c2090a4bde65e197b93ea
parent 109617 eaf3765dbed9e4f28798cb68696bf633531a5dad
child 109619 518ee633eb9cea07271e31e6f04d2bc43a5f7767
push id1579
push userjdrew@mozilla.com
push dateMon, 22 Oct 2012 18:53:02 +0000
treeherdermozilla-beta@aa46c50b7747 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlebar, lsblakk
bugs789046
milestone17.0
Bug 789046 - Port a bug fix from the Webkit GIF decoder back to its progenitor, Mozilla! r=jlebar a=lsblakk
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;