Fixed bad bitmap size crashes
authorcommit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Wed, 4 Dec 2013 17:06:49 +0000 (17:06 +0000)
committercommit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81>
Wed, 4 Dec 2013 17:06:49 +0000 (17:06 +0000)
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

20 files changed:
include/core/SkMallocPixelRef.h
include/core/SkPixelRef.h
samplecode/SampleFilterFuzz.cpp
src/core/SkBitmap.cpp
src/core/SkBitmapDevice.cpp
src/core/SkPixelRef.cpp
src/effects/SkBlurImageFilter.cpp
src/effects/SkColorFilterImageFilter.cpp
src/effects/SkDropShadowImageFilter.cpp
src/effects/SkLightingImageFilter.cpp
src/effects/SkMagnifierImageFilter.cpp
src/effects/SkMatrixConvolutionImageFilter.cpp
src/effects/SkMorphologyImageFilter.cpp
src/effects/SkOffsetImageFilter.cpp
src/effects/SkRectShaderImageFilter.cpp
src/effects/SkTileImageFilter.cpp
src/effects/SkXfermodeImageFilter.cpp
src/image/SkDataPixelRef.cpp
src/image/SkDataPixelRef.h
tests/SerializationTest.cpp

index 2241a51..100a15d 100644 (file)
@@ -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;
index d90e587..4c564e4 100644 (file)
@@ -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.
     */
index 3e3436f..d15c6eb 100644 (file)
 #include "SkPerlinNoiseShader.h"
 #include "SkRandom.h"
 #include "SkRectShaderImageFilter.h"
+#include "SkTileImageFilter.h"
 #include "SkView.h"
 #include "SkXfermodeImageFilter.h"
+#include <stdio.h>
+#include <time.h>
 
-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<float>(kBitmapSize))+offset),
-                          SkIntToScalar(R(static_cast<float>(kBitmapSize))+offset));
+static SkRect make_rect() {
+    return SkRect::MakeWH(SkIntToScalar(R(static_cast<float>(kBitmapSize))),
+                          SkIntToScalar(R(static_cast<float>(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<unsigned char*>(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<SkImageFilter*>(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);
 }
 
index 9d4aa87..ad840c4 100644 (file)
@@ -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;
             }
index 369f10c..2d7e413 100644 (file)
@@ -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() {
index 972474c..068513b 100644 (file)
@@ -247,6 +247,10 @@ SkData* SkPixelRef::onRefEncodedData() {
     return NULL;
 }
 
+size_t SkPixelRef::getAllocatedSizeInBytes() const {
+    return 0;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifdef SK_BUILD_FOR_ANDROID
index 9bceda7..2795f3a 100644 (file)
@@ -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);
index 2042c12..8b7b390 100755 (executable)
@@ -113,6 +113,9 @@ bool SkColorFilterImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& sourc
     }
 
     SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+    if (NULL == device.get()) {
+        return false;
+    }
     SkCanvas canvas(device.get());
     SkPaint paint;
 
index 5be633e..24a910d 100644 (file)
@@ -70,6 +70,9 @@ bool SkDropShadowImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& source
     }
 
     SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+    if (NULL == device.get()) {
+        return false;
+    }
     SkCanvas canvas(device.get());
 
     SkAutoTUnref<SkImageFilter> blurFilter(new SkBlurImageFilter(fSigmaX, fSigmaY));
index 8c8798f..4e3cee0 100644 (file)
@@ -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<SkLight> 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<SkLight> transformedLight(light()->transform(ctm));
index d412059..e6f3984 100644 (file)
@@ -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++;
index cef0450..3da27ce 100644 (file)
@@ -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,
index 3f2f985..0d00c35 100644 (file)
@@ -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();
index aefbcba..59318e3 100644 (file)
@@ -48,6 +48,9 @@ bool SkOffsetImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& source,
         }
 
         SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+        if (NULL == device.get()) {
+            return false;
+        }
         SkCanvas canvas(device);
         SkPaint paint;
         paint.setXfermodeMode(SkXfermode::kSrc_Mode);
index ab38fc4..5c34547 100644 (file)
@@ -62,6 +62,9 @@ bool SkRectShaderImageFilter::onFilterImage(Proxy* proxy,
 
     SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(),
                                                           bounds.height()));
+    if (NULL == device.get()) {
+        return false;
+    }
     SkCanvas canvas(device.get());
     SkPaint paint;
     paint.setShader(fShader);
index ccca4ff..7d3b72f 100644 (file)
@@ -38,6 +38,9 @@ bool SkTileImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& src, const S
     }
 
     SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(w, h));
+    if (NULL == device.get()) {
+        return false;
+    }
     SkIRect bounds;
     source.getBounds(&bounds);
     SkCanvas canvas(device);
index 620bde9..3ab5295 100644 (file)
@@ -72,6 +72,9 @@ bool SkXfermodeImageFilter::onFilterImage(Proxy* proxy,
     foregroundOffset.fY -= bounds.top();
 
     SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+    if (NULL == device.get()) {
+        return false;
+    }
     SkCanvas canvas(device);
     SkPaint paint;
     paint.setXfermodeMode(SkXfermode::kSrc_Mode);
index 0524243..7897bf9 100644 (file)
@@ -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);
index 6b15802..50c8857 100644 (file)
@@ -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;
index 9e63249..26c7fb0 100644 (file)
@@ -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<typename T> 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<SkMatrix> {
@@ -127,6 +139,45 @@ static void TestObjectSerialization(T* testObj, skiatest::Reporter* reporter) {
 }
 
 template<typename T>
+static T* TestFlattenableSerialization(T* testObj, bool shouldSucceed,
+                                       skiatest::Reporter* reporter) {
+    SkOrderedWriteBuffer writer(1024);
+    writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+    SerializationUtils<T>::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<T>::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<const unsigned char*>(buffer2.skip(0));
+    T* obj2 = NULL;
+    SerializationUtils<T>::Read(buffer2, &obj2);
+    const unsigned char* peekAfter = static_cast<const unsigned char*>(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<size_t>(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<typename T>
 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<SkXfermode> mode(SkXfermode::Create(SkXfermode::kSrcOver_Mode));
+    SkXfermodeImageFilter xfermodeImageFilter(mode, &invalidBitmapSource, &validBitmapSource);
+
+    SkAutoTUnref<SkImageFilter> deserializedFilter(
+        TestFlattenableSerialization<SkImageFilter>(
+            &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"