From 0354c62e0bb89607a29ce2150e77e3308a795cc4 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Fri, 24 Feb 2017 15:33:24 -0500 Subject: 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 Commit-Queue: Leon Scroggins --- 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 0000000000..4a08a61e01 Binary files /dev/null and b/resources/invalid_images/b34778578.bmp differ diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp index cd46a5495c..354bee6f74 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 64232712d7..265b51e886 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 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 stream(new SkFILEStream(path.c_str())); + if (!stream->isValid()) { + return; + } - std::unique_ptr 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 codec(SkCodec::NewFromStream(stream.release())); + REPORTER_ASSERT(r, !codec); + } } DEF_TEST(Codec_InvalidRLEBmp, r) { -- cgit v1.2.3