From cd3b15ca6364a04b0eeeb4f89c7daa8aefe854c8 Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Wed, 4 Dec 2013 17:06:49 +0000 Subject: [PATCH] Fixed bad bitmap size crashes There were 2 issues : 1 ) If the size of an SkBitmap's underlying SkPixelRef's alocated memory is too small to fit the bitmap, then the deserialization will now check this and set an error appropriately. 2 ) If a device fails to allocate its pixels, the device will be deleted and NULL will be returned to avoid attempting to draw on a bad device. BUG= R=senorblanco@chromium.org, reed@google.com, sugoi@google.com, halcanary@google.com, mtklein@google.com Author: sugoi@chromium.org Review URL: https://codereview.chromium.org/92793002 git-svn-id: http://skia.googlecode.com/svn/trunk@12484 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkMallocPixelRef.h | 5 +- include/core/SkPixelRef.h | 10 +++ samplecode/SampleFilterFuzz.cpp | 49 +++++++++--- src/core/SkBitmap.cpp | 5 ++ src/core/SkBitmapDevice.cpp | 20 ++++- src/core/SkPixelRef.cpp | 4 + src/effects/SkBlurImageFilter.cpp | 4 + src/effects/SkColorFilterImageFilter.cpp | 3 + src/effects/SkDropShadowImageFilter.cpp | 3 + src/effects/SkLightingImageFilter.cpp | 7 ++ src/effects/SkMagnifierImageFilter.cpp | 20 +++-- src/effects/SkMatrixConvolutionImageFilter.cpp | 3 + src/effects/SkMorphologyImageFilter.cpp | 6 ++ src/effects/SkOffsetImageFilter.cpp | 3 + src/effects/SkRectShaderImageFilter.cpp | 3 + src/effects/SkTileImageFilter.cpp | 3 + src/effects/SkXfermodeImageFilter.cpp | 3 + src/image/SkDataPixelRef.cpp | 4 + src/image/SkDataPixelRef.h | 1 + tests/SerializationTest.cpp | 103 +++++++++++++++++++++++++ 20 files changed, 237 insertions(+), 22 deletions(-) diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h index 2241a51..100a15d 100644 --- a/include/core/SkMallocPixelRef.h +++ b/include/core/SkMallocPixelRef.h @@ -24,8 +24,6 @@ public: SkMallocPixelRef(void* addr, size_t size, SkColorTable* ctable, bool ownPixels = true); virtual ~SkMallocPixelRef(); - //! Return the allocation size for the pixels - size_t getSize() const { return fSize; } void* getAddr() const { return fStorage; } SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkMallocPixelRef) @@ -38,6 +36,9 @@ protected: SkMallocPixelRef(SkFlattenableReadBuffer& buffer); virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE; + // Returns the allocation size for the pixels + virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE { return fSize; } + private: void* fStorage; size_t fSize; diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h index d90e587..4c564e4 100644 --- a/include/core/SkPixelRef.h +++ b/include/core/SkPixelRef.h @@ -257,6 +257,16 @@ protected: // default impl returns NULL. virtual SkData* onRefEncodedData(); + /** + * Returns the size (in bytes) of the internally allocated memory. + * This should be implemented in all serializable SkPixelRef derived classes. + * SkBitmap::fPixelRefOffset + SkBitmap::getSafeSize() should never overflow this value, + * otherwise the rendering code may attempt to read memory out of bounds. + * + * @return default impl returns 0. + */ + virtual size_t getAllocatedSizeInBytes() const; + /** Return the mutex associated with this pixelref. This value is assigned in the constructor, and cannot change during the lifetime of the object. */ diff --git a/samplecode/SampleFilterFuzz.cpp b/samplecode/SampleFilterFuzz.cpp index 3e3436f..d15c6eb 100644 --- a/samplecode/SampleFilterFuzz.cpp +++ b/samplecode/SampleFilterFuzz.cpp @@ -25,10 +25,13 @@ #include "SkPerlinNoiseShader.h" #include "SkRandom.h" #include "SkRectShaderImageFilter.h" +#include "SkTileImageFilter.h" #include "SkView.h" #include "SkXfermodeImageFilter.h" +#include +#include -static const uint32_t kSeed = 1; +static const uint32_t kSeed = (uint32_t)(time(NULL)); static SkRandom gRand(kSeed); static bool return_large = false; static bool return_undef = false; @@ -83,9 +86,9 @@ static SkScalar make_scalar(bool positiveOnly = false) { return make_number(positiveOnly); } -static SkRect make_rect(int offset = 1) { - return SkRect::MakeWH(SkIntToScalar(R(static_cast(kBitmapSize))+offset), - SkIntToScalar(R(static_cast(kBitmapSize))+offset)); +static SkRect make_rect() { + return SkRect::MakeWH(SkIntToScalar(R(static_cast(kBitmapSize))), + SkIntToScalar(R(static_cast(kBitmapSize)))); } static SkXfermode::Mode make_xfermode() { @@ -163,7 +166,7 @@ static SkImageFilter* make_image_filter(bool canBeNull = true) { enum { BICUBIC, MERGE, COLOR, BLUR, MAGNIFIER, XFERMODE, OFFSET, COMPOSE, DISTANT_LIGHT, POINT_LIGHT, SPOT_LIGHT, NOISE, DROP_SHADOW, - MORPHOLOGY, BITMAP, DISPLACE, NUM_FILTERS }; + MORPHOLOGY, BITMAP, DISPLACE, TILE, NUM_FILTERS }; switch (R(NUM_FILTERS)) { case BICUBIC: @@ -185,7 +188,7 @@ static SkImageFilter* make_image_filter(bool canBeNull = true) { filter = new SkBlurImageFilter(make_scalar(true), make_scalar(true), make_image_filter()); break; case MAGNIFIER: - filter = new SkMagnifierImageFilter(make_rect(0), make_scalar(true)); + filter = new SkMagnifierImageFilter(make_rect(), make_scalar(true)); break; case XFERMODE: { @@ -256,6 +259,9 @@ static SkImageFilter* make_image_filter(bool canBeNull = true) { make_channel_selector_type(), make_scalar(), make_image_filter(false), make_image_filter()); break; + case TILE: + filter = new SkTileImageFilter(make_rect(), make_rect(), make_image_filter(false)); + break; default: break; } @@ -270,14 +276,27 @@ static SkImageFilter* make_serialized_image_filter() { #ifdef SK_ADD_RANDOM_BIT_FLIPS unsigned char* p = const_cast(ptr); for (size_t i = 0; i < len; ++i, ++p) { - if ((R(1000) == 1)) { // 0.1% of the time, flip a bit - *p ^= (1 << R(8)); + if (R(250) == 1) { // 0.4% of the time, flip a bit or byte + if (R(10) == 1) { // Then 10% of the time, change a whole byte + switch(R(3)) { + case 0: + *p ^= 0xFF; // Flip entire byte + break; + case 1: + *p = 0xFF; // Set all bits to 1 + break; + case 2: + *p = 0x00; // Set all bits to 0 + break; + } + } else { + *p ^= (1 << R(8)); + } } } #endif // SK_ADD_RANDOM_BIT_FLIPS SkFlattenable* flattenable = SkValidatingDeserializeFlattenable(ptr, len, SkImageFilter::GetFlattenableType()); - SkASSERT(NULL != flattenable); return static_cast(flattenable); } @@ -290,8 +309,18 @@ static void drawClippedBitmap(SkCanvas* canvas, int x, int y, const SkPaint& pai } static void do_fuzz(SkCanvas* canvas) { +#ifdef SK_FUZZER_IS_VERBOSE + static uint32_t filterId = 0; + if (0 == filterId) { + printf("Fuzzing with %u\n", kSeed); + } + printf("Filter no %u\r", filterId); + fflush(stdout); + filterId++; +#endif + SkPaint paint; - paint.setImageFilter(make_serialized_image_filter())->unref(); + SkSafeUnref(paint.setImageFilter(make_serialized_image_filter())); drawClippedBitmap(canvas, 0, 0, paint); } diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 9d4aa87..ad840c4 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -1560,6 +1560,7 @@ void SkBitmap::unflatten(SkFlattenableReadBuffer& buffer) { SkIsValidConfig(config) && validate_alphaType(config, alphaType)); this->setConfig(config, width, height, rowBytes, alphaType); + buffer.validate(fRowBytes >= (fWidth * fBytesPerPixel)); int reftype = buffer.readInt(); if (buffer.validate((SERIALIZE_PIXELTYPE_REF_DATA == reftype) || @@ -1568,6 +1569,10 @@ void SkBitmap::unflatten(SkFlattenableReadBuffer& buffer) { case SERIALIZE_PIXELTYPE_REF_DATA: { size_t offset = buffer.readUInt(); SkPixelRef* pr = buffer.readPixelRef(); + if (!buffer.validate((NULL == pr) || + (pr->getAllocatedSizeInBytes() >= (offset + this->getSafeSize())))) { + offset = 0; + } SkSafeUnref(this->setPixelRef(pr, offset)); break; } diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 369f10c..2d7e413 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -29,7 +29,10 @@ SkBitmapDevice::SkBitmapDevice(const SkBitmap& bitmap, const SkDeviceProperties& SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque) { fBitmap.setConfig(config, width, height, 0, isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType); - fBitmap.allocPixels(); + if (!fBitmap.allocPixels()) { + fBitmap.setConfig(config, 0, 0, 0, isOpaque ? + kOpaque_SkAlphaType : kPremul_SkAlphaType); + } if (!isOpaque) { fBitmap.eraseColor(SK_ColorTRANSPARENT); } @@ -41,7 +44,10 @@ SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, b fBitmap.setConfig(config, width, height, 0, isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType); - fBitmap.allocPixels(); + if (!fBitmap.allocPixels()) { + fBitmap.setConfig(config, 0, 0, 0, isOpaque ? + kOpaque_SkAlphaType : kPremul_SkAlphaType); + } if (!isOpaque) { fBitmap.eraseColor(SK_ColorTRANSPARENT); } @@ -61,8 +67,14 @@ SkBaseDevice* SkBitmapDevice::onCreateCompatibleDevice(SkBitmap::Config config, int width, int height, bool isOpaque, Usage usage) { - return SkNEW_ARGS(SkBitmapDevice,(config, width, height, isOpaque, - this->getDeviceProperties())); + SkBitmapDevice* device = SkNEW_ARGS(SkBitmapDevice,(config, width, height, + isOpaque, this->getDeviceProperties())); + // Check if allocation failed and delete device if it did fail + if ((device->width() != width) || (device->height() != height)) { + SkDELETE(device); + device = NULL; + } + return device; } void SkBitmapDevice::lockPixels() { diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 972474c..068513b 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -247,6 +247,10 @@ SkData* SkPixelRef::onRefEncodedData() { return NULL; } +size_t SkPixelRef::getAllocatedSizeInBytes() const { + return 0; +} + /////////////////////////////////////////////////////////////////////////////// #ifdef SK_BUILD_FOR_ANDROID diff --git a/src/effects/SkBlurImageFilter.cpp b/src/effects/SkBlurImageFilter.cpp index 9bceda7..2795f3a 100644 --- a/src/effects/SkBlurImageFilter.cpp +++ b/src/effects/SkBlurImageFilter.cpp @@ -159,6 +159,10 @@ bool SkBlurImageFilter::onFilterImage(Proxy* proxy, dst->setConfig(src.config(), srcBounds.width(), srcBounds.height()); dst->getBounds(&dstBounds); dst->allocPixels(); + if (!dst->getPixels()) { + return false; + } + int kernelSizeX, kernelSizeX3, lowOffsetX, highOffsetX; int kernelSizeY, kernelSizeY3, lowOffsetY, highOffsetY; getBox3Params(fSigma.width(), &kernelSizeX, &kernelSizeX3, &lowOffsetX, &highOffsetX); diff --git a/src/effects/SkColorFilterImageFilter.cpp b/src/effects/SkColorFilterImageFilter.cpp index 2042c12..8b7b390 100755 --- a/src/effects/SkColorFilterImageFilter.cpp +++ b/src/effects/SkColorFilterImageFilter.cpp @@ -113,6 +113,9 @@ bool SkColorFilterImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& sourc } SkAutoTUnref device(proxy->createDevice(bounds.width(), bounds.height())); + if (NULL == device.get()) { + return false; + } SkCanvas canvas(device.get()); SkPaint paint; diff --git a/src/effects/SkDropShadowImageFilter.cpp b/src/effects/SkDropShadowImageFilter.cpp index 5be633e..24a910d 100644 --- a/src/effects/SkDropShadowImageFilter.cpp +++ b/src/effects/SkDropShadowImageFilter.cpp @@ -70,6 +70,9 @@ bool SkDropShadowImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& source } SkAutoTUnref device(proxy->createDevice(bounds.width(), bounds.height())); + if (NULL == device.get()) { + return false; + } SkCanvas canvas(device.get()); SkAutoTUnref blurFilter(new SkBlurImageFilter(fSigmaX, fSigmaY)); diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp index 8c8798f..4e3cee0 100644 --- a/src/effects/SkLightingImageFilter.cpp +++ b/src/effects/SkLightingImageFilter.cpp @@ -814,6 +814,7 @@ void SkLight::flattenLight(SkFlattenableWriteBuffer& buffer) const { case SkLight::kSpot_LightType: return SkNEW_ARGS(SkSpotLight, (buffer)); default: SkDEBUGFAIL("Unknown LightType."); + buffer.validate(false); return NULL; } } @@ -952,6 +953,9 @@ bool SkDiffuseLightingImageFilter::onFilterImage(Proxy* proxy, dst->setConfig(src.config(), bounds.width(), bounds.height()); dst->allocPixels(); + if (!dst->getPixels()) { + return false; + } SkAutoTUnref transformedLight(light()->transform(ctm)); @@ -1040,6 +1044,9 @@ bool SkSpecularLightingImageFilter::onFilterImage(Proxy* proxy, dst->setConfig(src.config(), bounds.width(), bounds.height()); dst->allocPixels(); + if (!dst->getPixels()) { + return false; + } SpecularLightingType lightingType(fKS, fShininess); SkAutoTUnref transformedLight(light()->transform(ctm)); diff --git a/src/effects/SkMagnifierImageFilter.cpp b/src/effects/SkMagnifierImageFilter.cpp index d412059..e6f3984 100644 --- a/src/effects/SkMagnifierImageFilter.cpp +++ b/src/effects/SkMagnifierImageFilter.cpp @@ -240,7 +240,9 @@ SkMagnifierImageFilter::SkMagnifierImageFilter(SkFlattenableReadBuffer& buffer) fSrcRect = SkRect::MakeXYWH(x, y, width, height); fInset = buffer.readScalar(); - buffer.validate(SkIsValidRect(fSrcRect) && SkScalarIsFinite(fInset)); + buffer.validate(SkScalarIsFinite(fInset) && SkIsValidRect(fSrcRect) && + // Negative numbers in src rect are not supported + (fSrcRect.fLeft >= 0) && (fSrcRect.fTop >= 0)); } // FIXME: implement single-input semantics @@ -283,7 +285,9 @@ bool SkMagnifierImageFilter::onFilterImage(Proxy*, const SkBitmap& src, SkASSERT(fSrcRect.width() < src.width()); SkASSERT(fSrcRect.height() < src.height()); - if (src.config() != SkBitmap::kARGB_8888_Config) { + if ((src.config() != SkBitmap::kARGB_8888_Config) || + (fSrcRect.width() >= src.width()) || + (fSrcRect.height() >= src.height())) { return false; } @@ -293,13 +297,17 @@ bool SkMagnifierImageFilter::onFilterImage(Proxy*, const SkBitmap& src, return false; } + dst->setConfig(src.config(), src.width(), src.height()); + dst->allocPixels(); + if (!dst->getPixels()) { + return false; + } + SkScalar inv_inset = fInset > 0 ? SkScalarInvert(fInset) : SK_Scalar1; SkScalar inv_x_zoom = fSrcRect.width() / src.width(); SkScalar inv_y_zoom = fSrcRect.height() / src.height(); - dst->setConfig(src.config(), src.width(), src.height()); - dst->allocPixels(); SkColor* sptr = src.getAddr32(0, 0); SkColor* dptr = dst->getAddr32(0, 0); int width = src.width(), height = src.height(); @@ -332,8 +340,8 @@ bool SkMagnifierImageFilter::onFilterImage(Proxy*, const SkBitmap& src, SkScalar y_interp = SkScalarMul(weight, (fSrcRect.y() + y * inv_y_zoom)) + (SK_Scalar1 - weight) * y; - int x_val = SkMin32(SkScalarFloorToInt(x_interp), width - 1); - int y_val = SkMin32(SkScalarFloorToInt(y_interp), height - 1); + int x_val = SkPin32(SkScalarFloorToInt(x_interp), 0, width - 1); + int y_val = SkPin32(SkScalarFloorToInt(y_interp), 0, height - 1); *dptr = sptr[y_val * width + x_val]; dptr++; diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp index cef0450..3da27ce 100644 --- a/src/effects/SkMatrixConvolutionImageFilter.cpp +++ b/src/effects/SkMatrixConvolutionImageFilter.cpp @@ -279,6 +279,9 @@ bool SkMatrixConvolutionImageFilter::onFilterImage(Proxy* proxy, result->setConfig(src.config(), bounds.width(), bounds.height()); result->allocPixels(); + if (!result->getPixels()) { + return false; + } SkIRect interior = SkIRect::MakeXYWH(bounds.left() + fTarget.fX, bounds.top() + fTarget.fY, diff --git a/src/effects/SkMorphologyImageFilter.cpp b/src/effects/SkMorphologyImageFilter.cpp index 3f2f985..0d00c35 100644 --- a/src/effects/SkMorphologyImageFilter.cpp +++ b/src/effects/SkMorphologyImageFilter.cpp @@ -188,6 +188,9 @@ bool SkErodeImageFilter::onFilterImage(Proxy* proxy, dst->setConfig(src.config(), bounds.width(), bounds.height()); dst->allocPixels(); + if (!dst->getPixels()) { + return false; + } int width = radius().width(); int height = radius().height(); @@ -247,6 +250,9 @@ bool SkDilateImageFilter::onFilterImage(Proxy* proxy, dst->setConfig(src.config(), bounds.width(), bounds.height()); dst->allocPixels(); + if (!dst->getPixels()) { + return false; + } int width = radius().width(); int height = radius().height(); diff --git a/src/effects/SkOffsetImageFilter.cpp b/src/effects/SkOffsetImageFilter.cpp index aefbcba..59318e3 100644 --- a/src/effects/SkOffsetImageFilter.cpp +++ b/src/effects/SkOffsetImageFilter.cpp @@ -48,6 +48,9 @@ bool SkOffsetImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& source, } SkAutoTUnref device(proxy->createDevice(bounds.width(), bounds.height())); + if (NULL == device.get()) { + return false; + } SkCanvas canvas(device); SkPaint paint; paint.setXfermodeMode(SkXfermode::kSrc_Mode); diff --git a/src/effects/SkRectShaderImageFilter.cpp b/src/effects/SkRectShaderImageFilter.cpp index ab38fc4..5c34547 100644 --- a/src/effects/SkRectShaderImageFilter.cpp +++ b/src/effects/SkRectShaderImageFilter.cpp @@ -62,6 +62,9 @@ bool SkRectShaderImageFilter::onFilterImage(Proxy* proxy, SkAutoTUnref device(proxy->createDevice(bounds.width(), bounds.height())); + if (NULL == device.get()) { + return false; + } SkCanvas canvas(device.get()); SkPaint paint; paint.setShader(fShader); diff --git a/src/effects/SkTileImageFilter.cpp b/src/effects/SkTileImageFilter.cpp index ccca4ff..7d3b72f 100644 --- a/src/effects/SkTileImageFilter.cpp +++ b/src/effects/SkTileImageFilter.cpp @@ -38,6 +38,9 @@ bool SkTileImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& src, const S } SkAutoTUnref device(proxy->createDevice(w, h)); + if (NULL == device.get()) { + return false; + } SkIRect bounds; source.getBounds(&bounds); SkCanvas canvas(device); diff --git a/src/effects/SkXfermodeImageFilter.cpp b/src/effects/SkXfermodeImageFilter.cpp index 620bde9..3ab5295 100644 --- a/src/effects/SkXfermodeImageFilter.cpp +++ b/src/effects/SkXfermodeImageFilter.cpp @@ -72,6 +72,9 @@ bool SkXfermodeImageFilter::onFilterImage(Proxy* proxy, foregroundOffset.fY -= bounds.top(); SkAutoTUnref device(proxy->createDevice(bounds.width(), bounds.height())); + if (NULL == device.get()) { + return false; + } SkCanvas canvas(device); SkPaint paint; paint.setXfermodeMode(SkXfermode::kSrc_Mode); diff --git a/src/image/SkDataPixelRef.cpp b/src/image/SkDataPixelRef.cpp index 0524243..7897bf9 100644 --- a/src/image/SkDataPixelRef.cpp +++ b/src/image/SkDataPixelRef.cpp @@ -27,6 +27,10 @@ void SkDataPixelRef::onUnlockPixels() { // nothing to do } +size_t SkDataPixelRef::getAllocatedSizeInBytes() const { + return fData ? fData->size() : 0; +} + void SkDataPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const { this->INHERITED::flatten(buffer); buffer.writeDataAsByteArray(fData); diff --git a/src/image/SkDataPixelRef.h b/src/image/SkDataPixelRef.h index 6b15802..50c8857 100644 --- a/src/image/SkDataPixelRef.h +++ b/src/image/SkDataPixelRef.h @@ -22,6 +22,7 @@ public: protected: virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; + virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE; SkDataPixelRef(SkFlattenableReadBuffer& buffer); virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE; diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp index 9e63249..26c7fb0 100644 --- a/tests/SerializationTest.cpp +++ b/tests/SerializationTest.cpp @@ -5,8 +5,13 @@ * found in the LICENSE file. */ +#include "SkBitmapDevice.h" +#include "SkBitmapSource.h" +#include "SkCanvas.h" +#include "SkMallocPixelRef.h" #include "SkOrderedWriteBuffer.h" #include "SkValidatingReadBuffer.h" +#include "SkXfermodeImageFilter.h" #include "Test.h" static const uint32_t kArraySize = 64; @@ -22,6 +27,13 @@ static void TestAlignment(T* testObj, skiatest::Reporter* reporter) { } template struct SerializationUtils { + // Generic case for flattenables + static void Write(SkOrderedWriteBuffer& writer, const T* flattenable) { + writer.writeFlattenable(flattenable); + } + static void Read(SkValidatingReadBuffer& reader, T** flattenable) { + *flattenable = (T*)reader.readFlattenable(T::GetFlattenableType()); + } }; template<> struct SerializationUtils { @@ -127,6 +139,45 @@ static void TestObjectSerialization(T* testObj, skiatest::Reporter* reporter) { } template +static T* TestFlattenableSerialization(T* testObj, bool shouldSucceed, + skiatest::Reporter* reporter) { + SkOrderedWriteBuffer writer(1024); + writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag); + SerializationUtils::Write(writer, testObj); + size_t bytesWritten = writer.bytesWritten(); + REPORTER_ASSERT(reporter, SkAlign4(bytesWritten) == bytesWritten); + + unsigned char dataWritten[1024]; + writer.writeToMemory(dataWritten); + + // Make sure this fails when it should (test with smaller size, but still multiple of 4) + SkValidatingReadBuffer buffer(dataWritten, bytesWritten - 4); + T* obj = NULL; + SerializationUtils::Read(buffer, &obj); + REPORTER_ASSERT(reporter, !buffer.validate(true)); + REPORTER_ASSERT(reporter, NULL == obj); + + // Make sure this succeeds when it should + SkValidatingReadBuffer buffer2(dataWritten, bytesWritten); + const unsigned char* peekBefore = static_cast(buffer2.skip(0)); + T* obj2 = NULL; + SerializationUtils::Read(buffer2, &obj2); + const unsigned char* peekAfter = static_cast(buffer2.skip(0)); + if (shouldSucceed) { + // This should have succeeded, since there are enough bytes to read this + REPORTER_ASSERT(reporter, buffer2.validate(true)); + REPORTER_ASSERT(reporter, static_cast(peekAfter - peekBefore) == bytesWritten); + REPORTER_ASSERT(reporter, NULL != obj2); + } else { + // If the deserialization was supposed to fail, make sure it did + REPORTER_ASSERT(reporter, !buffer.validate(true)); + REPORTER_ASSERT(reporter, NULL == obj2); + } + + return obj2; // Return object to perform further validity tests on it +} + +template static void TestArraySerialization(T* data, skiatest::Reporter* reporter) { SkOrderedWriteBuffer writer(1024); writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag); @@ -152,6 +203,35 @@ static void TestArraySerialization(T* data, skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, success); } +static void TestBitmapSerialization(const SkBitmap& validBitmap, + const SkBitmap& invalidBitmap, + bool shouldSucceed, + skiatest::Reporter* reporter) { + SkBitmapSource validBitmapSource(validBitmap); + SkBitmapSource invalidBitmapSource(invalidBitmap); + SkAutoTUnref mode(SkXfermode::Create(SkXfermode::kSrcOver_Mode)); + SkXfermodeImageFilter xfermodeImageFilter(mode, &invalidBitmapSource, &validBitmapSource); + + SkAutoTUnref deserializedFilter( + TestFlattenableSerialization( + &xfermodeImageFilter, shouldSucceed, reporter)); + + // Try to render a small bitmap using the invalid deserialized filter + // to make sure we don't crash while trying to render it + if (shouldSucceed) { + SkBitmap bitmap; + bitmap.setConfig(SkBitmap::kARGB_8888_Config, 24, 24); + bitmap.allocPixels(); + SkBitmapDevice device(bitmap); + SkCanvas canvas(&device); + canvas.clear(0x00000000); + SkPaint paint; + paint.setImageFilter(deserializedFilter); + canvas.clipRect(SkRect::MakeXYWH(0, 0, SkIntToScalar(24), SkIntToScalar(24))); + canvas.drawBitmap(bitmap, 0, 0, &paint); + } +} + static void Tests(skiatest::Reporter* reporter) { // Test matrix serialization { @@ -212,6 +292,29 @@ static void Tests(skiatest::Reporter* reporter) { SkScalar data[kArraySize] = { SK_Scalar1, SK_ScalarHalf, SK_ScalarMax }; TestArraySerialization(data, reporter); } + + // Test invalid deserializations + { + SkBitmap validBitmap; + validBitmap.setConfig(SkBitmap::kARGB_8888_Config, 256, 256); + + // Create a bitmap with a really large height + SkBitmap invalidBitmap; + invalidBitmap.setConfig(SkBitmap::kARGB_8888_Config, 256, 1000000000); + + // The deserialization should succeed, and the rendering shouldn't crash, + // even when the device fails to initialize, due to its size + TestBitmapSerialization(validBitmap, invalidBitmap, true, reporter); + + // Create a bitmap with a pixel ref too small + SkBitmap invalidBitmap2; + invalidBitmap2.setConfig(SkBitmap::kARGB_8888_Config, 256, 256); + invalidBitmap2.setPixelRef(SkNEW_ARGS(SkMallocPixelRef, + (NULL, 256, NULL)))->unref(); + + // The deserialization should detect the pixel ref being too small and fail + TestBitmapSerialization(validBitmap, invalidBitmap2, false, reporter); + } } #include "TestClassDef.h" -- 2.7.4