SkGifCodec: intersect frameRect with image size

When clearing due to SkCodecAnimation::RestoreBGColor_DisposalMethod,
intersect the frameRect with the image size to prevent clearing outside
the bounds of the allocated memory.

Add a test image, created by the fuzzer.

BUG=skia:6046

Change-Id: I43676d28f82abf093ef801752f3a9e881580924c
Reviewed-on: https://skia-review.googlesource.com/5860
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>

diff --git a/resources/invalid_images/skbug6046.gif b/resources/invalid_images/skbug6046.gif
new file mode 100644 (file)
index 0000000..fc9a9af
Binary files /dev/null and b/resources/invalid_images/skbug6046.gif differ

diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 9a5948b..618a5b5 100644 (file)
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -419,18 +419,20 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
             }
             const auto* prevFrame = fReader->frameContext(frameContext->getRequiredFrame());
             if (prevFrame->getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod) {
-                const SkIRect prevRect = prevFrame->frameRect();
-                auto left = get_scaled_dimension(prevRect.fLeft, fSwizzler->sampleX());
-                auto top = get_scaled_dimension(prevRect.fTop, fSwizzler->sampleY());
-                void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes
-                        + left * SkColorTypeBytesPerPixel(dstInfo.colorType()));
-                auto width = get_scaled_dimension(prevRect.width(), fSwizzler->sampleX());
-                auto height = get_scaled_dimension(prevRect.height(), fSwizzler->sampleY());
-                // fSwizzler->fill() would fill to the scaled width of the frame, but we want to
-                // fill to the scaled with of the width of the PRIOR frame, so we do all the scaling
-                // ourselves and call the static version.
-                SkSampler::Fill(dstInfo.makeWH(width, height), eraseDst,
-                                fDstRowBytes, this->getFillValue(dstInfo), kNo_ZeroInitialized);
+                SkIRect prevRect = prevFrame->frameRect();
+                if (prevRect.intersect(this->getInfo().bounds())) {
+                    auto left = get_scaled_dimension(prevRect.fLeft, fSwizzler->sampleX());
+                    auto top = get_scaled_dimension(prevRect.fTop, fSwizzler->sampleY());
+                    void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes
+                            + left * SkColorTypeBytesPerPixel(dstInfo.colorType()));
+                    auto width = get_scaled_dimension(prevRect.width(), fSwizzler->sampleX());
+                    auto height = get_scaled_dimension(prevRect.height(), fSwizzler->sampleY());
+                    // fSwizzler->fill() would fill to the scaled width of the frame, but we want to
+                    // fill to the scaled with of the width of the PRIOR frame, so we do all the
+                    // scaling ourselves and call the static version.
+                    SkSampler::Fill(dstInfo.makeWH(width, height), eraseDst,
+                                    fDstRowBytes, this->getFillValue(dstInfo), kNo_ZeroInitialized);
+                }
             }
             filledBackground = true;
         }

diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index bbe88d7..05e8c3f 100644 (file)
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1452,3 +1452,37 @@ DEF_TEST(Codec_InvalidImages, r) {
     test_invalid_images(r, "invalid_images/skbug5887.gif", true);
     test_invalid_images(r, "invalid_images/many-progressive-scans.jpg", false);
 }
+
+DEF_TEST(Codec_InvalidAnimated, r) {
+    // ASAN will complain if there is an issue.
+    auto path = "invalid_images/skbug6046.gif";
+    auto* stream = GetResourceAsStream(path);
+    if (!stream) {
+        return;
+    }
+
+    std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream));
+    REPORTER_ASSERT(r, codec);
+    if (!codec) {
+        return;
+    }
+
+    const auto info = codec->getInfo().makeColorType(kN32_SkColorType);
+    SkBitmap bm;
+    bm.allocPixels(info);
+
+    auto frameInfos = codec->getFrameInfo();
+    SkCodec::Options opts;
+    for (size_t i = 0; i < frameInfos.size(); i++) {
+        opts.fFrameIndex = i;
+        opts.fHasPriorFrame = frameInfos[i].fRequiredFrame == i - 1;
+        auto result = codec->startIncrementalDecode(info, bm.getPixels(), bm.rowBytes(), &opts);
+        if (result != SkCodec::kSuccess) {
+            ERRORF(r, "Failed to start decoding frame %i (out of %i) with error %i\n", i,
+                   frameInfos.size(), result);
+            continue;
+        }
+
+        codec->incrementalDecode();
+    }
+}