755523 - GIF decoder can read past end of heap buffer

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect)

Description

[Peter Kasting]

See http://code.google.com/p/chromium/issues/detail?id=128163 for the original report and analysis in Chromium.  In short, when a GIF specifies an extension block, the block often starts with a byte giving the length of the block; but the spec requires these lengths to be of particular values, and the code that decodes each particular block assumes sufficient data is available.  A malformed GIF can therefore specify too small of a value here, causing the decoder to attempt to read past the end of the GIF when decoding the block.

Mozilla suffers from this bug just as Chromium does; see http://mxr.mozilla.org/mozilla-central/source/image/decoders/nsGIFDecoder2.cpp#799 and following.

See https://bugs.webkit.org/show_bug.cgi?id=86531 for a patch to fix the Chromium GIF decoder, which is very similar to the Mozilla GIF decoder (since it was forked from it); this could be adapted to fix the Mozilla code as well.

Comment 1

[Brian R. Bondy [:bbondy]]

Perhaps you could attach the patch here?

When clicking on the bugs.webkit.org link I get:
You are not authorized to access bug #86531. To see this bug, you must first log in to an account with the appropriate permissions.

Also I can't access the chromium link.

Comment 2

[Al Billings [:abillings - ex-MoCo]]

Rating as a sec-moderate due to possible information leakage and/or disclosure.

Comment 3

[Peter Kasting]

Attachment: WebKit patch for reference

Sorry about that.  Here's the patch I landed on the WebKit tree.  The GIFImageReader.cpp changes are the interesting ones.

There was some debate about the correct fix here, since it's possible to fix this a couple different ways.

Comment 4

[Brian R. Bondy [:bbondy]]

Thanks for attaching it :)

Comment 5

[Peter Kasting]

Attachment: Second WebKit patch (fixes bustage in first)

Turns out my first fix broke things.  Apparently there are GIFs that try to embed ICC profiles but do it in a way that violates the GIF spec.  I'm attaching a patch-against-the-first-patch that allows these to still display while fixing the original memory error.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

This appears to be a duplicate of Bug 789046, but I don't have access to it, so I can't check or mark it a duplicate.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

See bug 844684 - it appears we are covering for these cases.