From 0354c62e0bb89607a29ce2150e77e3308a795cc4 Mon Sep 17 00:00:00 2001
From: Leon Scroggins III <scroggo@google.com>
Date: Fri, 24 Feb 2017 15:33:24 -0500
Subject: [PATCH] Set a limit on the size for BMP images

This limit matches the limit used by Chromium. I am not aware of any
real world BMPs that are larger than this (or even close to it), but
there are some invalid BMPs that are larger than this, leading to
crashes when we try to read a row.

BUG:34778578
BUG=skia:3617

Change-Id: I0f662e8d0d7bc0b084e86d0c9288b831e1b296d7
Reviewed-on: https://skia-review.googlesource.com/8966
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
---
 resources/invalid_images/b34778578.bmp | Bin 0 -> 132 bytes
 src/codec/SkBmpCodec.cpp               |   7 ++++---
 tests/CodecTest.cpp                    |  25 +++++++++++++------------
 3 files changed, 17 insertions(+), 15 deletions(-)
 create mode 100644 resources/invalid_images/b34778578.bmp

diff --git a/resources/invalid_images/b34778578.bmp b/resources/invalid_images/b34778578.bmp
new file mode 100644
index 0000000000000000000000000000000000000000..4a08a61e0100e09f5d60125e275bfcb4fead1434
GIT binary patch
literal 132
zcmZ?rJ=gHRKCYgDp^SmyfCd8t10%!#`oI7GzhYz%0P=u9g@Hkcn}LG|$P@w#a01CT
yppL6x@c%!AzIfG7hM@r{)pGUf%d38O7#gl#-3O7vAU!?*yD>EEbn^rPYfk{9%sVOo

literal 0
HcmV?d00001

diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index cd46a54..354bee6 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -280,9 +280,10 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
     if (inIco) {
         height /= 2;
     }
-    if (width <= 0 || height <= 0) {
-        // TODO: Decide if we want to disable really large bmps as well.
-        // https://code.google.com/p/skia/issues/detail?id=3617
+
+    // Arbitrary maximum. Matches Chromium.
+    constexpr int kMaxDim = 1 << 16;
+    if (width <= 0 || height <= 0 || width >= kMaxDim || height >= kMaxDim) {
         SkCodecPrintf("Error: invalid bitmap dimensions.\n");
         return false;
     }
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 6423271..265b51e 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -17,6 +17,7 @@
 #include "SkFrontBufferedStream.h"
 #include "SkImageEncoder.h"
 #include "SkMD5.h"
+#include "SkOSPath.h"
 #include "SkPngChunkReader.h"
 #include "SkRandom.h"
 #include "SkStream.h"
@@ -1466,19 +1467,19 @@ DEF_TEST(Codec_InvalidImages, r) {
 }
 
 DEF_TEST(Codec_InvalidBmp, r) {
-    // This file reports a header size that crashes when we try to read this
-    // much directly from a file using SkFILEStream.
-    SkString path = GetResourcePath("invalid_images/b33651913.bmp");
-    std::unique_ptr<SkFILEStream> stream(new SkFILEStream(path.c_str()));
-    if (!stream->isValid()) {
-        ERRORF(r, "no stream");
-        return;
-    }
+    // These files report values that have caused problems with SkFILEStreams.
+    // They are invalid, and should not create SkCodecs.
+    for (auto* bmp : { "b33651913.bmp", "b34778578.bmp" } ) {
+        SkString path = SkOSPath::Join("invalid_images", bmp);
+        path = GetResourcePath(path.c_str());
+        std::unique_ptr<SkFILEStream> stream(new SkFILEStream(path.c_str()));
+        if (!stream->isValid()) {
+            return;
+        }
 
-    std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
-    // This file is invalid, but more importantly, we did not crash before
-    // reaching here.
-    REPORTER_ASSERT(r, !codec);
+        std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
+        REPORTER_ASSERT(r, !codec);
+    }
 }
 
 DEF_TEST(Codec_InvalidRLEBmp, r) {
-- 
2.7.4