Defend against ICOs with large BMPs embedded DO NOT MERGE If the ICO reports that it has a large BMP file embedded, do not crash if we attempt to allocate too much memory. Bug: 38116746 Merged-In: Ie6665dc8ade3beb19c276c4d48d1fabc077a2e6c Change-Id: I70eb66f5e4ffc15587007b398bbe843665eae500 Reviewed-on: https://skia-review.googlesource.com/18447 Reviewed-by: Matt Sarett <msarett@google.com> Commit-Queue: Leon Scroggins <scroggo@google.com> (cherry picked from commit 6029322ad78e0120cce18a01ae74e082c0e542fe)
diff --git a/resources/invalid_images/b38116746.ico b/resources/invalid_images/b38116746.ico new file mode 100644 index 0000000..35ee5b5 --- /dev/null +++ b/resources/invalid_images/b38116746.ico Binary files differ
diff --git a/src/codec/SkIcoCodec.cpp b/src/codec/SkIcoCodec.cpp index dc4222a..8b3d26d 100644 --- a/src/codec/SkIcoCodec.cpp +++ b/src/codec/SkIcoCodec.cpp
@@ -14,6 +14,7 @@ #include "SkStream.h" #include "SkTDArray.h" #include "SkTSort.h" +#include "../private/SkTemplates.h" /* * Checks the start of the stream to see if the image is an Ico or Cur @@ -128,12 +129,18 @@ bytesRead = offset; // Create a new stream for the embedded codec - SkAutoTUnref<SkData> data( - SkData::NewFromStream(inputStream.get(), size)); - if (nullptr == data.get()) { + SkAutoFree buffer(sk_malloc_flags(size, 0)); + if (!buffer.get()) { + SkCodecPrintf("Warning: OOM trying to create embedded stream.\n"); + break; + } + + if (inputStream->read(buffer.get(), size) != size) { SkCodecPrintf("Warning: could not create embedded stream.\n"); break; } + + SkAutoTUnref<SkData> data(SkData::NewFromMalloc(buffer.detach(), size)); SkAutoTDelete<SkMemoryStream> embeddedStream(new SkMemoryStream(data.get())); bytesRead += size;
diff --git a/tests/BadIcoTest.cpp b/tests/BadIcoTest.cpp index c387e15..f6b1c46 100644 --- a/tests/BadIcoTest.cpp +++ b/tests/BadIcoTest.cpp
@@ -22,6 +22,7 @@ "ico_fuzz1.ico", "skbug3442.webp", "skbug3429.webp", + "b38116746.ico", }; const char* badImagesFolder = "invalid_images";