commit cbf66a22130a1fd13285bb65d3a0d7ee6b4e8ab3
author Leon Scroggins III <scroggo@google.com> Fri Feb 16 12:03:03 2018 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Feb 16 17:26:56 2018 +0000
tree 37e60202849dfce25b2a9a8e204ce0cb730da84c
parent 50d4a6a661909a36d259e3c6007337af8050a9fe

Ensure all rows of a gif are initialized

Bug: oss-fuzz:6274

Even if a frame does not have enough LZW blocks to decode all rows,
(which is unknown until we actually decode them), it is marked complete
once there are no more LZW blocks.

When decoding, even if we've decoded all LZW blocks, check fRowsDecoded
to determine whether we've actually all the rows. Report the number of
rows decoded so that SkCodec can fill in the remaining ones.

Change-Id: I1d6e0c29e3c37649725836cf24a4a239e3266b76
Reviewed-on: https://skia-review.googlesource.com/106964
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>

resources/invalid_images/ossfuzz6274.gif

diff --git a/resources/invalid_images/ossfuzz6274.gif b/resources/invalid_images/ossfuzz6274.gif
new file mode 100644
index 0000000..faa3b6e
--- /dev/null
+++ b/resources/invalid_images/ossfuzz6274.gif
Binary files differ

src/codec/SkGifCodec.cpp

diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 1118ed0..dbff928 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -332,6 +332,8 @@
 
 SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts, int* rowsDecoded) {
     const SkImageInfo& dstInfo = this->dstInfo();
+    const int scaledHeight = get_scaled_dimension(dstInfo.height(), fSwizzler->sampleY());
+
     const int frameIndex = opts.fFrameIndex;
     SkASSERT(frameIndex < fReader->imagesCount());
     const SkGIFFrameContext* frameContext = fReader->frameContext(frameIndex);
@@ -354,8 +356,6 @@
                     || frameContext->interlaced() || !fCurrColorTableIsReal) {
                 // fill ignores the width (replaces it with the actual, scaled width).
                 // But we need to scale in Y.
-                const int scaledHeight = get_scaled_dimension(dstInfo.height(),
-                                                              fSwizzler->sampleY());
                 auto fillInfo = dstInfo.makeWH(0, scaledHeight);
                 fSwizzler->fill(fillInfo, fDst, fDstRowBytes, this->getFillValue(dstInfo),
                                 opts.fZeroInitialized);
@@ -370,7 +370,7 @@
         fFilledBackground = filledBackground;
         if (filledBackground) {
             // Report the full (scaled) height, since the client will never need to fill.
-            fRowsDecoded = get_scaled_dimension(dstInfo.height(), fSwizzler->sampleY());
+            fRowsDecoded = scaledHeight;
         } else {
             // This will be updated by haveDecodedRow.
             fRowsDecoded = 0;
@@ -384,7 +384,7 @@
 
     bool frameDecoded = false;
     const bool fatalError = !fReader->decode(frameIndex, &frameDecoded);
-    if (fatalError || !frameDecoded) {
+    if (fatalError || !frameDecoded || fRowsDecoded != scaledHeight) {
         if (rowsDecoded) {
             *rowsDecoded = fRowsDecoded;
         }

tests/CodecTest.cpp

diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 290686f..6e0a88c 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -10,6 +10,7 @@
 #include "SkAndroidCodec.h"
 #include "SkAutoMalloc.h"
 #include "SkBitmap.h"
+#include "SkCanvas.h"
 #include "SkCodec.h"
 #include "SkCodecImageGenerator.h"
 #include "SkColorSpace_XYZ.h"
@@ -27,6 +28,7 @@
 #include "SkRandom.h"
 #include "SkStream.h"
 #include "SkStreamPriv.h"
+#include "SkUnPreMultiply.h"
 #include "SkWebpEncoder.h"
 #include "Test.h"
 
@@ -1528,3 +1530,38 @@
 
     test_info(r, codec.get(), codec->getInfo(), SkCodec::kInvalidInput, nullptr);
 }
+
+DEF_TEST(Codec_ossfuzz6274, r) {
+    if (GetResourcePath().isEmpty()) {
+        return;
+    }
+
+    const char* file = "invalid_images/ossfuzz6274.gif";
+    auto image = GetResourceAsImage(file);
+    if (!image) {
+        ERRORF(r, "Missing %s", file);
+        return;
+    }
+
+    REPORTER_ASSERT(r, image->width()  == 32);
+    REPORTER_ASSERT(r, image->height() == 32);
+
+    SkBitmap bm;
+    if (!bm.tryAllocPixels(SkImageInfo::MakeN32Premul(32, 32))) {
+        ERRORF(r, "Failed to allocate pixels");
+        return;
+    }
+
+    bm.eraseColor(SK_ColorTRANSPARENT);
+
+    SkCanvas canvas(bm);
+    canvas.drawImage(image, 0, 0, nullptr);
+
+    for (int i = 0; i < image->width();  ++i)
+    for (int j = 0; j < image->height(); ++j) {
+        SkColor actual = SkUnPreMultiply::PMColorToColor(*bm.getAddr32(i, j));
+        if (actual != SK_ColorTRANSPARENT) {
+            ERRORF(r, "did not initialize pixels! %i, %i is %x", i, j, actual);
+        }
+    }
+}