aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Matt Sarett <msarett@google.com>2016-10-17 14:32:46 -0400
committerGravatar Mike Klein <mtklein@chromium.org>2016-10-17 21:19:54 +0000
commit29121ebd9e4673d7ca6ac1ba3dd905fbdc9d50aa (patch)
treecf85bf7e983c484137f566edd6970cbbae0b88a0
parentc03e1c55a79f00d02ab528945425ff50cb700402 (diff)
Avoid integer overflow in SkIcoCodec and SkImageInfo
Original Commit Message: """ Avoid integer overflow in SkIcoCodec Definitely good to avoid overflow here. FWIW, this looks to be harmless for Android's current use. They will just fail later on when trying to allocate the bitmap. BUG=skia:5857 """ With the new test, ASAN also caught an integer overflow bug in SkImageInfo. Fix this as well. CQ_INCLUDE_TRYBOTS=master.client.skia:Test-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Debug-ASAN;master.client.skia.android:Test-Android-Clang-AndroidOne-CPU-MT6582-arm-Debug-GN_Android BUG=skia:5857 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3568 Change-Id: I0d777a547850474ea6cea87e36efa05434e33635 Reviewed-on: https://skia-review.googlesource.com/3568 Reviewed-by: Kevin Lubick <kjlubick@google.com>
-rw-r--r--include/core/SkImageInfo.h8
-rw-r--r--resources/invalid_images/int_overflow.icobin0 -> 323 bytes
-rw-r--r--src/codec/SkIcoCodec.cpp9
-rw-r--r--tests/CodecTest.cpp7
4 files changed, 18 insertions, 6 deletions
diff --git a/include/core/SkImageInfo.h b/include/core/SkImageInfo.h
index cf50acacae..8498f583fa 100644
--- a/include/core/SkImageInfo.h
+++ b/include/core/SkImageInfo.h
@@ -277,7 +277,11 @@ public:
}
size_t minRowBytes() const {
- return (size_t)this->minRowBytes64();
+ uint64_t minRowBytes = this->minRowBytes64();
+ if (!sk_64_isS32(minRowBytes)) {
+ return 0;
+ }
+ return sk_64_asS32(minRowBytes);
}
size_t computeOffset(int x, int y, size_t rowBytes) const {
@@ -302,7 +306,7 @@ public:
if (0 == fHeight) {
return 0;
}
- return sk_64_mul(fHeight - 1, rowBytes) + fWidth * this->bytesPerPixel();
+ return sk_64_mul(fHeight - 1, rowBytes) + sk_64_mul(fWidth, this->bytesPerPixel());
}
size_t getSafeSize(size_t rowBytes) const {
diff --git a/resources/invalid_images/int_overflow.ico b/resources/invalid_images/int_overflow.ico
new file mode 100644
index 0000000000..ba675666b6
--- /dev/null
+++ b/resources/invalid_images/int_overflow.ico
Binary files differ
diff --git a/src/codec/SkIcoCodec.cpp b/src/codec/SkIcoCodec.cpp
index 63b72c4039..d01904dc1b 100644
--- a/src/codec/SkIcoCodec.cpp
+++ b/src/codec/SkIcoCodec.cpp
@@ -157,11 +157,12 @@ SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
}
// Use the largest codec as a "suggestion" for image info
- uint32_t maxSize = 0;
- uint32_t maxIndex = 0;
- for (int32_t i = 0; i < codecs->count(); i++) {
+ size_t maxSize = 0;
+ int maxIndex = 0;
+ for (int i = 0; i < codecs->count(); i++) {
SkImageInfo info = codecs->operator[](i)->getInfo();
- uint32_t size = info.width() * info.height();
+ size_t size = info.getSafeSize(info.minRowBytes());
+
if (size > maxSize) {
maxSize = size;
maxIndex = i;
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 4d18c61860..a6058a9c35 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1362,3 +1362,10 @@ DEF_TEST(Codec_rowsDecoded, r) {
REPORTER_ASSERT(r, result == SkCodec::kIncompleteInput);
REPORTER_ASSERT(r, rowsDecoded == 0);
}
+
+DEF_TEST(Codec_IcoIntOverflow, r) {
+ // ASAN will complain if there is an issue.
+ SkBitmap bitmap;
+ const bool success = GetResourceAsBitmap("invalid_images/int_overflow.ico", &bitmap);
+ REPORTER_ASSERT(r, !success);
+}