diff options
author | Leon Scroggins III <scroggo@google.com> | 2016-12-12 17:10:46 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2016-12-13 14:00:17 +0000 |
commit | 56e3209c70ef50ed7e39a0150d94e057695bf6b6 (patch) | |
tree | b5758186dc612dfd31780fb714d38fbed11ad676 | |
parent | be3bdd9000cfab825c8d9ea7126abefd0665ab3a (diff) |
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>
-rw-r--r-- | resources/invalid_images/skbug6046.gif | bin | 0 -> 72 bytes | |||
-rw-r--r-- | src/codec/SkGifCodec.cpp | 26 | ||||
-rw-r--r-- | tests/CodecTest.cpp | 34 |
3 files changed, 48 insertions, 12 deletions
diff --git a/resources/invalid_images/skbug6046.gif b/resources/invalid_images/skbug6046.gif Binary files differnew file mode 100644 index 0000000000..fc9a9af1ba --- /dev/null +++ b/resources/invalid_images/skbug6046.gif diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index 9a5948b2d0..618a5b5302 100644 --- 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 bbe88d7d3d..05e8c3f4e8 100644 --- 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(); + } +} |