Validate readByteArrayAsData size

Check that the reader has enough data before attempting to allocate the
buffer.

Also update to return nullptr on read failures.

Change-Id: Ia1ea8f611bad95cf3a4493b12582ac3fa7c2b00f
Reviewed-on: https://skia-review.googlesource.com/127129
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>

6 files changed, 27 insertions, 19 deletions

diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index f28843e0d6..780cb13349 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -233,6 +233,7 @@ void SkPicturePlayback::handleOp(SkReadBuffer* reader,
             reader->readString(&key);
             sk_sp<SkData> data = reader->readByteArrayAsData();
             BREAK_ON_READ_ERROR(reader);
+            SkASSERT(data);
 
             canvas->drawAnnotation(rect, key.c_str(), data.get());
         } break;
diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp
index 48b6881b0a..adf9e64403 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -5,7 +5,9 @@
  * found in the LICENSE file.
  */
 
+#include "SkAutoMalloc.h"
 #include "SkBitmap.h"
+#include "SkData.h"
 #include "SkDeduper.h"
 #include "SkImage.h"
 #include "SkImageGenerator.h"
@@ -260,6 +262,20 @@ bool SkReadBuffer::readScalarArray(SkScalar* values, size_t size) {
     return this->readArray(values, size, sizeof(SkScalar));
 }
 
+sk_sp<SkData> SkReadBuffer::readByteArrayAsData() {
+    size_t numBytes = this->getArrayCount();
+    if (!this->validate(fReader.isAvailable(numBytes))) {
+        return nullptr;
+    }
+
+    SkAutoMalloc buffer(numBytes);
+    if (!this->readByteArray(buffer.get(), numBytes)) {
+        return nullptr;
+    }
+
+    return SkData::MakeFromMalloc(buffer.release(), numBytes);
+}
+
 uint32_t SkReadBuffer::getArrayCount() {
     const size_t inc = sizeof(uint32_t);
     fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h
index ec64e8a0b2..fd6f5d279a 100644
--- a/src/core/SkReadBuffer.h
+++ b/src/core/SkReadBuffer.h
@@ -9,7 +9,6 @@
 #define SkReadBuffer_DEFINED
 
 #include "SkColorFilter.h"
-#include "SkData.h"
 #include "SkSerialProcs.h"
 #include "SkDrawLooper.h"
 #include "SkImageFilter.h"
@@ -24,6 +23,7 @@
 #include "SkTHash.h"
 #include "SkWriteBuffer.h"
 
+class SkData;
 class SkImage;
 class SkInflator;
 
@@ -168,15 +168,7 @@ public:
     bool readPointArray(SkPoint* points, size_t size);
     bool readScalarArray(SkScalar* values, size_t size);
 
-    sk_sp<SkData> readByteArrayAsData() {
-        size_t len = this->getArrayCount();
-        void* buffer = sk_malloc_throw(len);
-        if (!this->readByteArray(buffer, len)) {
-            sk_free(buffer);
-            return SkData::MakeEmpty();
-        }
-        return SkData::MakeFromMalloc(buffer, len);
-    }
+    sk_sp<SkData> readByteArrayAsData();
 
     // helpers to get info about arrays and binary data
     uint32_t getArrayCount();
diff --git a/src/effects/SkToSRGBColorFilter.cpp b/src/effects/SkToSRGBColorFilter.cpp
index 0e9a8c98e1..d2020ef338 100644
--- a/src/effects/SkToSRGBColorFilter.cpp
+++ b/src/effects/SkToSRGBColorFilter.cpp
@@ -64,10 +64,7 @@ SkToSRGBColorFilter::SkToSRGBColorFilter(sk_sp<SkColorSpace> srcColorSpace)
 
 sk_sp<SkFlattenable> SkToSRGBColorFilter::CreateProc(SkReadBuffer& buffer) {
     auto data = buffer.readByteArrayAsData();
-    if (data) {
-        return Make(SkColorSpace::Deserialize(data->data(), data->size()));
-    }
-    return nullptr;
+    return data ? Make(SkColorSpace::Deserialize(data->data(), data->size())) : nullptr;
 }
 
 void SkToSRGBColorFilter::flatten(SkWriteBuffer& buffer) const {
diff --git a/src/pipe/SkPipeReader.cpp b/src/pipe/SkPipeReader.cpp
index 2614c4ecd5..ada4a21342 100644
--- a/src/pipe/SkPipeReader.cpp
+++ b/src/pipe/SkPipeReader.cpp
@@ -562,8 +562,10 @@ static void drawImageLattice_handler(SkPipeReader& reader, uint32_t packedVerb,
 static void drawVertices_handler(SkPipeReader& reader, uint32_t packedVerb, SkCanvas* canvas) {
     SkASSERT(SkPipeVerb::kDrawVertices == unpack_verb(packedVerb));
     SkBlendMode bmode = (SkBlendMode)unpack_verb_extra(packedVerb);
-    sk_sp<SkData> data = reader.readByteArrayAsData();
-    canvas->drawVertices(SkVertices::Decode(data->data(), data->size()), bmode, read_paint(reader));
+    if (sk_sp<SkData> data = reader.readByteArrayAsData()) {
+        canvas->drawVertices(SkVertices::Decode(data->data(), data->size()), bmode,
+                             read_paint(reader));
+    }
 }
 
 static void drawPicture_handler(SkPipeReader& reader, uint32_t packedVerb, SkCanvas* canvas) {
@@ -634,7 +636,7 @@ static void defineImage_handler(SkPipeReader& reader, uint32_t packedVerb, SkCan
     } else {
         // we are defining a new image
         sk_sp<SkData> data = reader.readByteArrayAsData();
-        sk_sp<SkImage> image = inflator->makeImage(data);
+        sk_sp<SkImage> image = data ? inflator->makeImage(data) : nullptr;
         if (!image) {
             SkDebugf("-- failed to decode\n");
         }
@@ -663,7 +665,7 @@ static void defineTypeface_handler(SkPipeReader& reader, uint32_t packedVerb, Sk
         // we are defining a new image
         sk_sp<SkData> data = reader.readByteArrayAsData();
         // TODO: seems like we could "peek" to see the array, and not need to copy it.
-        sk_sp<SkTypeface> tf = inflator->makeTypeface(data->data(), data->size());
+        sk_sp<SkTypeface> tf = data ? inflator->makeTypeface(data->data(), data->size()) : nullptr;
         inflator->setTypeface(index, tf.get());
     }
 }
diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index f61bb7c2d4..7f597cbac2 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -102,7 +102,7 @@ bool SkGradientShaderBase::DescriptorScope::unflatten(SkReadBuffer& buffer) {
 
     if (SkToBool(flags & kHasColorSpace_GSF)) {
         sk_sp<SkData> data = buffer.readByteArrayAsData();
-        fColorSpace = SkColorSpace::Deserialize(data->data(), data->size());
+        fColorSpace = data ? SkColorSpace::Deserialize(data->data(), data->size()) : nullptr;
     } else {
         fColorSpace = nullptr;
     }