Malformed GIF can cause decoder to read off end of heap buffer

https://bugs.webkit.org/show_bug.cgi?id=86531

Reviewed by Adam Barth.

Source/WebCore:

Test: fast/images/read-past-end-of-buffer.html
This test is only expected to catch problems if run under Address
Sanitizer or a similar memory-checking utility.

* platform/image-decoders/gif/GIFImageReader.cpp:
(GIFImageReader::read):

LayoutTests:

The test here is only expected to catch problems if run under Address
Sanitizer or a similar memory-checking utility.

* fast/images/read-past-end-of-buffer-expected.txt: Added.
* fast/images/read-past-end-of-buffer.html: Added.
* fast/images/resources/wrong-block-length.gif: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@117333 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 144ef12..2b14c7e 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2012-05-15  Peter Kasting  <pkasting@google.com>
+
+        Malformed GIF can cause decoder to read off end of heap buffer
+        https://bugs.webkit.org/show_bug.cgi?id=86531
+
+        Reviewed by Adam Barth.
+
+        The test here is only expected to catch problems if run under Address
+        Sanitizer or a similar memory-checking utility.
+
+        * fast/images/read-past-end-of-buffer-expected.txt: Added.
+        * fast/images/read-past-end-of-buffer.html: Added.
+        * fast/images/resources/wrong-block-length.gif: Added.
+
 2012-05-16  Alexis Menard  <alexis.menard@openbossa.org>
 
         [Qt] REGRESSION?(62951): media tests fail

diff --git a/LayoutTests/fast/images/read-past-end-of-buffer-expected.txt b/LayoutTests/fast/images/read-past-end-of-buffer-expected.txt
new file mode 100644 (file)
index 0000000..3fd0353
--- /dev/null
+++ b/LayoutTests/fast/images/read-past-end-of-buffer-expected.txt
@@ -0,0 +1,2 @@
+This test passes if no heap errors are detected during decoding.
+

diff --git a/LayoutTests/fast/images/read-past-end-of-buffer.html b/LayoutTests/fast/images/read-past-end-of-buffer.html
new file mode 100644 (file)
index 0000000..432a224
--- /dev/null
+++ b/LayoutTests/fast/images/read-past-end-of-buffer.html
@@ -0,0 +1,6 @@
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+This test passes if no heap errors are detected during decoding.<br>
+<img src=resources/wrong-block-length.gif>

diff --git a/LayoutTests/fast/images/resources/wrong-block-length.gif b/LayoutTests/fast/images/resources/wrong-block-length.gif
new file mode 100644 (file)
index 0000000..cbd3f6e
Binary files /dev/null and b/LayoutTests/fast/images/resources/wrong-block-length.gif differ

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index c3b5585..2817841 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2012-05-15  Peter Kasting  <pkasting@google.com>
+
+        Malformed GIF can cause decoder to read off end of heap buffer
+        https://bugs.webkit.org/show_bug.cgi?id=86531
+
+        Reviewed by Adam Barth.
+
+        Test: fast/images/read-past-end-of-buffer.html
+        This test is only expected to catch problems if run under Address
+        Sanitizer or a similar memory-checking utility.
+
+        * platform/image-decoders/gif/GIFImageReader.cpp:
+        (GIFImageReader::read):
+
 2012-05-16  Varun Jain  <varunjain@google.com>
 
         [chromium] No modifier flags (shift/ctrl/alt) in drag&drop events on chromium linux

diff --git a/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp b/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp
index 420b355..73f7d38 100644 (file)
--- a/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp
+++ b/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp
@@ -581,21 +581,30 @@ bool GIFImageReader::read(const unsigned char *buf, unsigned len,
 
     case gif_extension:
     {
-      int len = count = q[1];
+      count = q[1];
       gstate es = gif_skip_block;
 
+      // NOTE: We use the spec-mandated block lengths below where applicable,
+      // instead of the value in q[1]. If the two disagree, the GIF is
+      // malformed; the main thing we want to avoid is having the GIF specify a
+      // smaller number in q[1] than the spec requires and then having our
+      // processing code for these extensions blindly read off the end of a
+      // too-short heap buffer.
       switch (*q)
       {
       case 0xf9:
         es = gif_control_extension;
+        count = 4;
         break;
 
       case 0x01:
         // ignoring plain text extension
+        count = 12;
         break;
 
       case 0xff:
         es = gif_application_extension;
+        count = 11;
         break;
 
       case 0xfe:
@@ -603,8 +612,8 @@ bool GIFImageReader::read(const unsigned char *buf, unsigned len,
         break;
       }
 
-      if (len)
-        GETN(len, es);
+      if (count)
+        GETN(count, es);
       else
         GETN(1, gif_image_start);
     }