Adding size parameter to read array functions

In some cases, the allocated array into which the data will be read is using getArrayCount() to allocate itself, which should be safe, but some cases use fixed length arrays or compute the array size before reading, which could overflow if the stream is compromised.

To prevent that from happening, I added a check that will verify that the number of bytes to read will not exceed the capacity of the input buffer argument passed to all the read...Array() functions.

I chose to use the byte array for this initial version, so that "size" represents the same value across all read...Array() functions, but I could also use the element count, if it is preferred.

Note : readPointArray and writePointArray are unused, so I could also remove them

BUG=
R=reed@google.com, mtklein@google.com, senorblanco@chromium.org

Author: sugoi@chromium.org

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

git-svn-id: http://skia.googlecode.com/svn/trunk@12058 2bbb7eff-a529-9590-31e7-b0007b416f81

10 files changed, 20 insertions, 19 deletions

diff --git a/src/effects/SkBicubicImageFilter.cpp b/src/effects/SkBicubicImageFilter.cpp
index 778df3fd4f..d6674153d5 100644
--- a/src/effects/SkBicubicImageFilter.cpp
+++ b/src/effects/SkBicubicImageFilter.cpp
@@ -41,8 +41,8 @@ SkBicubicImageFilter* SkBicubicImageFilter::CreateMitchell(const SkSize& scale,
 }
 
 SkBicubicImageFilter::SkBicubicImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
-    SkDEBUGCODE(uint32_t readSize =) buffer.readScalarArray(fCoefficients);
-    SkASSERT(readSize == 16);
+    SkDEBUGCODE(bool success =) buffer.readScalarArray(fCoefficients, 16);
+    SkASSERT(success);
     fScale.fWidth = buffer.readScalar();
     fScale.fHeight = buffer.readScalar();
     buffer.validate(SkScalarIsFinite(fScale.fWidth) &&
diff --git a/src/effects/SkColorMatrixFilter.cpp b/src/effects/SkColorMatrixFilter.cpp
index f7b283e01f..d8eb1f17c5 100644
--- a/src/effects/SkColorMatrixFilter.cpp
+++ b/src/effects/SkColorMatrixFilter.cpp
@@ -308,7 +308,7 @@ void SkColorMatrixFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
 SkColorMatrixFilter::SkColorMatrixFilter(SkFlattenableReadBuffer& buffer)
         : INHERITED(buffer) {
     SkASSERT(buffer.getArrayCount() == 20);
-    buffer.readScalarArray(fMatrix.fMat);
+    buffer.readScalarArray(fMatrix.fMat, 20);
     this->initState(fMatrix.fMat);
     for (int i = 0; i < 20; ++i) {
         buffer.validate(SkScalarIsFinite(fMatrix.fMat[i]));
diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp
index 4aa46ab6bf..4099058502 100644
--- a/src/effects/SkDashPathEffect.cpp
+++ b/src/effects/SkDashPathEffect.cpp
@@ -555,5 +555,5 @@ SkDashPathEffect::SkDashPathEffect(SkFlattenableReadBuffer& buffer) : INHERITED(
 
     fCount = buffer.getArrayCount();
     fIntervals = (SkScalar*)sk_malloc_throw(sizeof(SkScalar) * fCount);
-    buffer.readScalarArray(fIntervals);
+    buffer.readScalarArray(fIntervals, fCount);
 }
diff --git a/src/effects/SkEmbossMaskFilter.cpp b/src/effects/SkEmbossMaskFilter.cpp
index 31beb511d5..2d3125000c 100644
--- a/src/effects/SkEmbossMaskFilter.cpp
+++ b/src/effects/SkEmbossMaskFilter.cpp
@@ -132,7 +132,7 @@ bool SkEmbossMaskFilter::filterMask(SkMask* dst, const SkMask& src,
 SkEmbossMaskFilter::SkEmbossMaskFilter(SkFlattenableReadBuffer& buffer)
         : SkMaskFilter(buffer) {
     SkASSERT(buffer.getArrayCount() == sizeof(Light));
-    buffer.readByteArray(&fLight);
+    buffer.readByteArray(&fLight, sizeof(Light));
     SkASSERT(fLight.fPad == 0); // for the font-cache lookup to be clean
     fBlurSigma = buffer.readScalar();
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V13_AND_ALL_OTHER_INSTANCES_TOO
diff --git a/src/effects/SkKernel33MaskFilter.cpp b/src/effects/SkKernel33MaskFilter.cpp
index 485001bbe3..821eebd785 100644
--- a/src/effects/SkKernel33MaskFilter.cpp
+++ b/src/effects/SkKernel33MaskFilter.cpp
@@ -120,8 +120,8 @@ void SkKernel33MaskFilter::flatten(SkFlattenableWriteBuffer& wb) const {
 
 SkKernel33MaskFilter::SkKernel33MaskFilter(SkFlattenableReadBuffer& rb)
         : SkKernel33ProcMaskFilter(rb) {
-    SkDEBUGCODE(const uint32_t count = )rb.readIntArray(&fKernel[0][0]);
-    SkASSERT(9 == count);
+    SkDEBUGCODE(bool success = )rb.readIntArray(&fKernel[0][0], 9);
+    SkASSERT(success);
     fShift = rb.readInt();
 }
 
diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp
index cac30e6a49..4864aec65a 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -59,18 +59,20 @@ SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(
 
 SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(SkFlattenableReadBuffer& buffer)
     : INHERITED(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
-        (SK_MaxS32 / fKernelSize.fWidth >= fKernelSize.fHeight)) {
-        uint32_t size = fKernelSize.fWidth * fKernelSize.fHeight;
+        (kMaxSize / fKernelSize.fWidth >= fKernelSize.fHeight)) {
+        size_t size = fKernelSize.fWidth * fKernelSize.fHeight;
         fKernel = SkNEW_ARRAY(SkScalar, size);
-        uint32_t readSize = buffer.readScalarArray(fKernel);
-        SkASSERT(readSize == size);
-        buffer.validate(readSize == size);
+        SkDEBUGCODE(bool success =) buffer.readScalarArray(fKernel, size);
+        SkASSERT(success);
     } else {
         fKernel = 0;
     }
diff --git a/src/effects/SkMergeImageFilter.cpp b/src/effects/SkMergeImageFilter.cpp
index 4de1093612..93e2335610 100755
--- a/src/effects/SkMergeImageFilter.cpp
+++ b/src/effects/SkMergeImageFilter.cpp
@@ -161,10 +161,9 @@ SkMergeImageFilter::SkMergeImageFilter(SkFlattenableReadBuffer& buffer) : INHERI
     if (hasModes) {
         this->initAllocModes();
         int nbInputs = countInputs();
-        bool sizeMatches = buffer.getArrayCount() == nbInputs * sizeof(fModes[0]);
-        buffer.validate(sizeMatches);
-        SkASSERT(sizeMatches);
-        buffer.readByteArray(fModes);
+        size_t size = nbInputs * sizeof(fModes[0]);
+        SkASSERT(buffer.getArrayCount() == size);
+        buffer.readByteArray(fModes, size);
         for (int i = 0; i < nbInputs; ++i) {
             buffer.validate(SkIsValidMode((SkXfermode::Mode)fModes[i]));
         }
diff --git a/src/effects/SkTableColorFilter.cpp b/src/effects/SkTableColorFilter.cpp
index 083b54c488..e15baf6928 100644
--- a/src/effects/SkTableColorFilter.cpp
+++ b/src/effects/SkTableColorFilter.cpp
@@ -189,7 +189,7 @@ SkTable_ColorFilter::SkTable_ColorFilter(SkFlattenableReadBuffer& buffer) : INHE
 
     size_t size = buffer.getArrayCount();
     SkASSERT(size <= sizeof(storage));
-    buffer.readByteArray(storage);
+    buffer.readByteArray(storage, size);
 
     SkDEBUGCODE(size_t raw = ) SkPackBits::Unpack8(storage, size, fStorage);
 
diff --git a/src/effects/SkTableMaskFilter.cpp b/src/effects/SkTableMaskFilter.cpp
index 5bff4def29..8d3b81a311 100644
--- a/src/effects/SkTableMaskFilter.cpp
+++ b/src/effects/SkTableMaskFilter.cpp
@@ -77,7 +77,7 @@ void SkTableMaskFilter::flatten(SkFlattenableWriteBuffer& wb) const {
 SkTableMaskFilter::SkTableMaskFilter(SkFlattenableReadBuffer& rb)
         : INHERITED(rb) {
     SkASSERT(256 == rb.getArrayCount());
-    rb.readByteArray(fTable);
+    rb.readByteArray(fTable, 256);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index c90adfeca0..2776199346 100644
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -159,7 +159,7 @@ SkGradientShaderBase::SkGradientShaderBase(SkFlattenableReadBuffer& buffer) : IN
     } else {
         fOrigColors = fStorage;
     }
-    buffer.readColorArray(fOrigColors);
+    buffer.readColorArray(fOrigColors, colorCount);
 
     {
         uint32_t packed = buffer.readUInt();