Sanitize SkMatrixConvolutionImageFilter creation params.

Apply the same memory limit in the Create() function that we do when
deserializing.

R=reed@google.com

Author: senorblanco@chromium.org

Review URL: https://codereview.chromium.org/610723002

include/effects/SkMatrixConvolutionImageFilter.h

@@ -61,11 +61,7 @@ public:
                                                   bool convolveAlpha,
                                                   SkImageFilter* input = NULL,
                                                   const CropRect* cropRect = NULL,
-                                                  uint32_t uniqueID = 0) {
-        return SkNEW_ARGS(SkMatrixConvolutionImageFilter, (kernelSize, kernel, gain, bias,
-                                                           kernelOffset, tileMode, convolveAlpha,
-                                                           input, cropRect, uniqueID));
-    }
+                                                  uint32_t uniqueID = 0);
 
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkMatrixConvolutionImageFilter)
 

src/effects/SkMatrixConvolutionImageFilter.cpp

@@ -17,6 +17,10 @@
 #include "effects/GrMatrixConvolutionEffect.h"
 #endif
 
+// We need to be able to read at most SK_MaxS32 bytes, so divide that
+// by the size of a scalar to know how many scalars we can read.
+static const int32_t gMaxKernelSize = SK_MaxS32 / sizeof(SkScalar);
+
 SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(
     const SkISize& kernelSize,
     const SkScalar* kernel,
@@ -35,7 +39,7 @@ SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(
     fKernelOffset(kernelOffset),
     fTileMode(tileMode),
     fConvolveAlpha(convolveAlpha) {
-    uint32_t size = fKernelSize.fWidth * fKernelSize.fHeight;
+    size_t size = (size_t) sk_64_mul(fKernelSize.width(), fKernelSize.height());
     fKernel = SkNEW_ARRAY(SkScalar, size);
     memcpy(fKernel, kernel, size * sizeof(SkScalar));
     SkASSERT(kernelSize.fWidth >= 1 && kernelSize.fHeight >= 1);
@@ -43,6 +47,31 @@ SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(
     SkASSERT(kernelOffset.fY >= 0 && kernelOffset.fY < kernelSize.fHeight);
 }
 
+SkMatrixConvolutionImageFilter* SkMatrixConvolutionImageFilter::Create(
+    const SkISize& kernelSize,
+    const SkScalar* kernel,
+    SkScalar gain,
+    SkScalar bias,
+    const SkIPoint& kernelOffset,
+    TileMode tileMode,
+    bool convolveAlpha,
+    SkImageFilter* input,
+    const CropRect* cropRect,
+    uint32_t uniqueID) {
+    if (kernelSize.width() < 1 || kernelSize.height() < 1) {
+        return NULL;
+    }
+    if (gMaxKernelSize / kernelSize.fWidth < kernelSize.fHeight) {
+        return NULL;
+    }
+    if (!kernel) {
+        return NULL;
+    }
+    return SkNEW_ARGS(SkMatrixConvolutionImageFilter, (kernelSize, kernel, gain, bias,
+                                                       kernelOffset, tileMode, convolveAlpha,
+                                                       input, cropRect, uniqueID));
+}
+
 #ifdef SK_SUPPORT_LEGACY_DEEPFLATTENING
 static bool tile_mode_is_valid(SkMatrixConvolutionImageFilter::TileMode tileMode) {
     switch (tileMode) {
@@ -58,16 +87,13 @@ static bool tile_mode_is_valid(SkMatrixConvolutionImageFilter::TileMode tileMode
 
 SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(SkReadBuffer& buffer)
     : INHERITED(1, buffer) {
-    // We need to be able to read at most SK_MaxS32 bytes, so divide that
-    // by the size of a scalar to know how many scalars we can read.
-    static const int32_t kMaxSize = SK_MaxS32 / sizeof(SkScalar);
     fKernelSize.fWidth = buffer.readInt();
     fKernelSize.fHeight = buffer.readInt();
     if ((fKernelSize.fWidth >= 1) && (fKernelSize.fHeight >= 1) &&
         // Make sure size won't be larger than a signed int,
         // which would still be extremely large for a kernel,
         // but we don't impose a hard limit for kernel size
-        (kMaxSize / fKernelSize.fWidth >= fKernelSize.fHeight)) {
+        (gMaxKernelSize / fKernelSize.fWidth >= fKernelSize.fHeight)) {
         size_t size = fKernelSize.fWidth * fKernelSize.fHeight;
         fKernel = SkNEW_ARRAY(SkScalar, size);
         SkDEBUGCODE(bool success =) buffer.readScalarArray(fKernel, size);

tests/ImageFilterTest.cpp

@@ -895,6 +895,60 @@ DEF_TEST(HugeBlurImageFilter, reporter) {
     test_huge_blur(&device, reporter);
 }
 
+DEF_TEST(MatrixConvolutionSanityTest, reporter) {
+    SkScalar kernel[1] = { 0 };
+    SkScalar gain = SK_Scalar1, bias = 0;
+    SkIPoint kernelOffset = SkIPoint::Make(1, 1);
+
+    // Check that an enormous (non-allocatable) kernel gives a NULL filter.
+    SkAutoTUnref<SkImageFilter> conv(SkMatrixConvolutionImageFilter::Create(
+        SkISize::Make(1<<30, 1<<30),
+        kernel,
+        gain,
+        bias,
+        kernelOffset,
+        SkMatrixConvolutionImageFilter::kRepeat_TileMode,
+        false));
+
+    REPORTER_ASSERT(reporter, NULL == conv.get());
+
+    // Check that a NULL kernel gives a NULL filter.
+    conv.reset(SkMatrixConvolutionImageFilter::Create(
+        SkISize::Make(1, 1),
+        NULL,
+        gain,
+        bias,
+        kernelOffset,
+        SkMatrixConvolutionImageFilter::kRepeat_TileMode,
+        false));
+
+    REPORTER_ASSERT(reporter, NULL == conv.get());
+
+    // Check that a kernel width < 1 gives a NULL filter.
+    conv.reset(SkMatrixConvolutionImageFilter::Create(
+        SkISize::Make(0, 1),
+        kernel,
+        gain,
+        bias,
+        kernelOffset,
+        SkMatrixConvolutionImageFilter::kRepeat_TileMode,
+        false));
+
+    REPORTER_ASSERT(reporter, NULL == conv.get());
+
+    // Check that kernel height < 1 gives a NULL filter.
+    conv.reset(SkMatrixConvolutionImageFilter::Create(
+        SkISize::Make(1, -1),
+        kernel,
+        gain,
+        bias,
+        kernelOffset,
+        SkMatrixConvolutionImageFilter::kRepeat_TileMode,
+        false));
+
+    REPORTER_ASSERT(reporter, NULL == conv.get());
+}
+
 static void test_xfermode_cropped_input(SkBaseDevice* device, skiatest::Reporter* reporter) {
     SkCanvas canvas(device);
     canvas.clear(0);