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>
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index f28843e..780cb13 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -233,6 +233,7 @@
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 48b6881..adf9e64 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 @@
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 ec64e8a..fd6f5d2 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 @@
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 0e9a8c9..d2020ef 100644
--- a/src/effects/SkToSRGBColorFilter.cpp
+++ b/src/effects/SkToSRGBColorFilter.cpp
@@ -64,10 +64,7 @@
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 2614c4e..ada4a21 100644
--- a/src/pipe/SkPipeReader.cpp
+++ b/src/pipe/SkPipeReader.cpp
@@ -562,8 +562,10 @@
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 @@
} 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 @@
// 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 f61bb7c..7f597cb 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -102,7 +102,7 @@
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;
}