Follow up to serialization validation code

src/effects/SkMatrixConvolutionImageFilter.cpp

 /*
  * Copyright 2012 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkMatrixConvolutionImageFilter.h"
 #include "SkBitmap.h"
 #include "SkColorPriv.h"
@@ skipping 43 matching lines @@
     memcpy(fKernel, kernel, size * sizeof(SkScalar));
     SkASSERT(kernelSize.fWidth >= 1 && kernelSize.fHeight >= 1);
     SkASSERT(target.fX >= 0 && target.fX < kernelSize.fWidth);
     SkASSERT(target.fY >= 0 && target.fY < kernelSize.fHeight);
 }
 
 SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(SkFlattenableReadBuffer& buffer)
     : INHERITED(buffer) {
     fKernelSize.fWidth = buffer.readInt();
     fKernelSize.fHeight = buffer.readInt();
-    uint32_t size = fKernelSize.fWidth * fKernelSize.fHeight;
-    fKernel = SkNEW_ARRAY(SkScalar, size);
-    SkDEBUGCODE(uint32_t readSize = )buffer.readScalarArray(fKernel);
-    SkASSERT(readSize == size);
+    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
+        (SK_MaxS32 / fKernelSize.fWidth >= fKernelSize.fHeight)) {
+        uint32_t size = fKernelSize.fWidth * fKernelSize.fHeight;
+        fKernel = SkNEW_ARRAY(SkScalar, size);
+        uint32_t readSize = buffer.readScalarArray(fKernel);

[Stephen White, 2013/10/23 18:09:35]

Shouldn't we be passing an expected size to readScalarArray() to avoid
overflowing fKernel?

[sugoi1, 2013/10/23 18:17:08]

Yes, a few of these functions need to be fixed (all the read...Array() functions
can overflow the input buffer and readPath, readRegion and readMatrix can
overflow the stream). Will do in the next cl.

+        SkASSERT(readSize == size);
+        buffer.validate(readSize == size);
+    } else {
+        fKernel = 0;
+    }
     fGain = buffer.readScalar();
     fBias = buffer.readScalar();
     fTarget.fX = buffer.readInt();
     fTarget.fY = buffer.readInt();
     fTileMode = (TileMode) buffer.readInt();
     fConvolveAlpha = buffer.readBool();
-    buffer.validate(SkScalarIsFinite(fGain) &&
+    buffer.validate((fKernel != 0) &&
+                    SkScalarIsFinite(fGain) &&
                     SkScalarIsFinite(fBias) &&
                     tile_mode_is_valid(fTileMode));
 }
 
 void SkMatrixConvolutionImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
     this->INHERITED::flatten(buffer);
     buffer.writeInt(fKernelSize.fWidth);
     buffer.writeInt(fKernelSize.fHeight);
     buffer.writeScalarArray(fKernel, fKernelSize.fWidth * fKernelSize.fHeight);
     buffer.writeScalar(fGain);
@@ skipping 560 matching lines @@
                                                 fBias,
                                                 fTarget,
                                                 fTileMode,
                                                 fConvolveAlpha);
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #endif