commit 4cdbf6056de29e8c12c3b1b4c2c2fa286cf68049
author Leon Scroggins III <scroggo@google.com> Thu Sep 28 14:33:57 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Sep 28 19:51:32 2017 +0000
tree a01bcff9249a9b5daada122ea46c8c7b21aef63b
parent c3269aea5ba61a0c2228b029c7626802474825f4

Avoid uninitialized memory in readByteArrayAsData

Bug: 769134

readByteArray can fail (due to not having enough available or due to the
wrong alignment). If it does, do not return an uninitialized block of
memory.

Further, drop the initial size check, which is covered by readByteArray.

Add a test.

Change-Id: Ia101697c5bb1ca3ae3df1795f37a74b2f602797d
Reviewed-on: https://skia-review.googlesource.com/52742
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

resources/crbug769134.fil

diff --git a/resources/crbug769134.fil b/resources/crbug769134.fil
new file mode 100644
index 0000000..a8a79e8
--- /dev/null
+++ b/resources/crbug769134.fil
Binary files differ

src/core/SkReadBuffer.h

diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h
index 0653ab6..980e8cf 100644
--- a/src/core/SkReadBuffer.h
+++ b/src/core/SkReadBuffer.h
@@ -165,11 +165,11 @@
 
     sk_sp<SkData> readByteArrayAsData() {
         size_t len = this->getArrayCount();
-        if (!this->validateAvailable(len)) {
+        void* buffer = sk_malloc_throw(len);
+        if (!this->readByteArray(buffer, len)) {
+            sk_free(buffer);
             return SkData::MakeEmpty();
         }
-        void* buffer = sk_malloc_throw(len);
-        this->readByteArray(buffer, len);
         return SkData::MakeFromMalloc(buffer, len);
     }
 

tests/ImageFilterTest.cpp

diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index db269f0..c39cc2e 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -38,6 +38,7 @@
 #include "SkTableColorFilter.h"
 #include "SkTileImageFilter.h"
 #include "SkXfermodeImageFilter.h"
+#include "Resources.h"
 #include "Test.h"
 #include "sk_tool_utils.h"
 
@@ -1717,6 +1718,18 @@
     REPORTER_ASSERT(reporter, *bm.getAddr32(0, 0) == SkPreMultiplyColor(SK_ColorGREEN));
 }
 
+DEF_TEST(ImageFilterImageSourceUninitialized, r) {
+    sk_sp<SkData> data(GetResourceAsData("crbug769134.fil"));
+    if (!data) {
+        return;
+    }
+    sk_sp<SkImageFilter> unflattenedFilter = SkValidatingDeserializeImageFilter(data->data(),
+                                                                                data->size());
+    // This will fail. More importantly, msan will verify that we did not
+    // compare against uninitialized memory.
+    REPORTER_ASSERT(r, !unflattenedFilter);
+}
+
 static void test_large_blur_input(skiatest::Reporter* reporter, SkCanvas* canvas) {
     SkBitmap largeBmp;
     int largeW = 5000;