Title: [117373] trunk/Source/WebCore
- Revision
- 117373
- Author
- pkast...@chromium.org
- Date
- 2012-05-16 18:17:46 -0700 (Wed, 16 May 2012)
Log Message
Correctly display malformed GIFs which specify bogus extension block
sizes.
https://bugs.webkit.org/show_bug.cgi?id=86531
Reviewed by James Robinson.
This was broken by r117333, which was an attempt to avoid memory errors
on GIFs that were malformed in a similar way. It turns out some GIFs
in the wild (i.e. "our LayoutTests directory") relied on some of the
effects of the old code. This refixes in a way that doesn't break
these.
No new tests, covered by existing tests.
* platform/image-decoders/gif/GIFImageReader.cpp:
(GIFImageReader::read):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (117372 => 117373)
--- trunk/Source/WebCore/ChangeLog 2012-05-17 01:14:23 UTC (rev 117372)
+++ trunk/Source/WebCore/ChangeLog 2012-05-17 01:17:46 UTC (rev 117373)
@@ -1,3 +1,22 @@
+2012-05-16 Peter Kasting <pkast...@google.com>
+
+ Correctly display malformed GIFs which specify bogus extension block
+ sizes.
+ https://bugs.webkit.org/show_bug.cgi?id=86531
+
+ Reviewed by James Robinson.
+
+ This was broken by r117333, which was an attempt to avoid memory errors
+ on GIFs that were malformed in a similar way. It turns out some GIFs
+ in the wild (i.e. "our LayoutTests directory") relied on some of the
+ effects of the old code. This refixes in a way that doesn't break
+ these.
+
+ No new tests, covered by existing tests.
+
+ * platform/image-decoders/gif/GIFImageReader.cpp:
+ (GIFImageReader::read):
+
2012-05-16 Chris Rogers <crog...@google.com>
AudioParam must support fan-in (multiple audio connections)
Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp (117372 => 117373)
--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp 2012-05-17 01:14:23 UTC (rev 117372)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp 2012-05-17 01:17:46 UTC (rev 117373)
@@ -584,27 +584,36 @@
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.
+ // 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.)
switch (*q)
{
case 0xf9:
es = gif_control_extension;
- count = 4;
+ count = std::max(count, 4);
break;
case 0x01:
// ignoring plain text extension
- count = 12;
+ count = std::max(count, 12);
break;
case 0xff:
es = gif_application_extension;
- count = 11;
+ count = std::max(count, 11);
break;
case 0xfe:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
