Adding more validation

- Added a way to check that the number of inputs of a filter is not more than a filter expects
- Added validation of reftype in SkBitmap::unflatten()
- Added validation on fKD (diffuse lighting constant) and fKS (specular lighting constant) to make sure that they are always non-negative numbers
- Added validation of SkPerlinNoiseShader::fType and SkPerlinNoiseShader::fNumOctaves

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

Author: sugoi@chromium.org

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

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

20 files changed, 79 insertions, 48 deletions

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 2fae75ae38..429d09216a 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -1622,19 +1622,21 @@ void SkBitmap::unflatten(SkFlattenableReadBuffer& buffer) {
     this->setConfig(config, width, height, rowBytes, alphaType);
 
     int reftype = buffer.readInt();
-    switch (reftype) {
-        case SERIALIZE_PIXELTYPE_REF_DATA: {
-            size_t offset = buffer.readUInt();
-            SkPixelRef* pr = buffer.readPixelRef();
-            SkSafeUnref(this->setPixelRef(pr, offset));
-            break;
+    if (buffer.validate((SERIALIZE_PIXELTYPE_REF_DATA == reftype) ||
+                        (SERIALIZE_PIXELTYPE_NONE == reftype))) {
+        switch (reftype) {
+            case SERIALIZE_PIXELTYPE_REF_DATA: {
+                size_t offset = buffer.readUInt();
+                SkPixelRef* pr = buffer.readPixelRef();
+                SkSafeUnref(this->setPixelRef(pr, offset));
+                break;
+            }
+            case SERIALIZE_PIXELTYPE_NONE:
+                break;
+            default:
+                SkDEBUGFAIL("unrecognized pixeltype in serialized data");
+                sk_throw();
         }
-        case SERIALIZE_PIXELTYPE_NONE:
-            break;
-        default:
-            buffer.validate(false);
-            SkDEBUGFAIL("unrecognized pixeltype in serialized data");
-            sk_throw();
     }
 }
 
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 9bf392515d..cca22bba55 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -53,20 +53,27 @@ SkImageFilter::~SkImageFilter() {
     delete[] fInputs;
 }
 
-SkImageFilter::SkImageFilter(SkFlattenableReadBuffer& buffer)
-    : fInputCount(buffer.readInt()), fInputs(new SkImageFilter*[fInputCount]) {
-    for (int i = 0; i < fInputCount; i++) {
-        if (buffer.readBool()) {
-            fInputs[i] = buffer.readImageFilter();
-        } else {
-            fInputs[i] = NULL;
+SkImageFilter::SkImageFilter(int maxInputCount, SkFlattenableReadBuffer& buffer) {
+    fInputCount = buffer.readInt();
+    if (buffer.validate((fInputCount >= 0) && (fInputCount <= maxInputCount))) {
+        fInputs = new SkImageFilter*[fInputCount];
+        for (int i = 0; i < fInputCount; i++) {
+            if (buffer.readBool()) {
+                fInputs[i] = buffer.readImageFilter();
+            } else {
+                fInputs[i] = NULL;
+            }
+        }
+        SkRect rect;
+        buffer.readRect(&rect);
+        if (buffer.validate(SkIsValidRect(rect))) {
+            uint32_t flags = buffer.readUInt();
+            fCropRect = CropRect(rect, flags);
         }
+    } else {
+        fInputCount = 0;
+        fInputs = NULL;
     }
-    SkRect rect;
-    buffer.readRect(&rect);
-    uint32_t flags = buffer.readUInt();
-    fCropRect = CropRect(rect, flags);
-    buffer.validate(SkIsValidRect(rect));
 }
 
 void SkImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
diff --git a/src/effects/SkBicubicImageFilter.cpp b/src/effects/SkBicubicImageFilter.cpp
index 5148e13e12..87ff046fc1 100644
--- a/src/effects/SkBicubicImageFilter.cpp
+++ b/src/effects/SkBicubicImageFilter.cpp
@@ -40,7 +40,8 @@ SkBicubicImageFilter* SkBicubicImageFilter::CreateMitchell(const SkSize& scale,
     return SkNEW_ARGS(SkBicubicImageFilter, (scale, gMitchellCoefficients, input));
 }
 
-SkBicubicImageFilter::SkBicubicImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkBicubicImageFilter::SkBicubicImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(1, buffer) {
     SkDEBUGCODE(bool success =) buffer.readScalarArray(fCoefficients, 16);
     SkASSERT(success);
     fScale.fWidth = buffer.readScalar();
diff --git a/src/effects/SkBitmapSource.cpp b/src/effects/SkBitmapSource.cpp
index 854df9df22..72f51f8423 100644
--- a/src/effects/SkBitmapSource.cpp
+++ b/src/effects/SkBitmapSource.cpp
@@ -8,12 +8,12 @@
 #include "SkBitmapSource.h"
 
 SkBitmapSource::SkBitmapSource(const SkBitmap& bitmap)
-  : INHERITED(0),
+  : INHERITED(0, 0),
     fBitmap(bitmap) {
 }
 
 SkBitmapSource::SkBitmapSource(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(0, buffer) {
     fBitmap.unflatten(buffer);
 }
 
diff --git a/src/effects/SkBlurImageFilter.cpp b/src/effects/SkBlurImageFilter.cpp
index 262733a74b..9bceda7db8 100644
--- a/src/effects/SkBlurImageFilter.cpp
+++ b/src/effects/SkBlurImageFilter.cpp
@@ -17,7 +17,7 @@
 #endif
 
 SkBlurImageFilter::SkBlurImageFilter(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(1, buffer) {
     fSigma.fWidth = buffer.readScalar();
     fSigma.fHeight = buffer.readScalar();
     buffer.validate(SkScalarIsFinite(fSigma.fWidth) &&
diff --git a/src/effects/SkColorFilterImageFilter.cpp b/src/effects/SkColorFilterImageFilter.cpp
index fdcc7fff6b..2042c12632 100755
--- a/src/effects/SkColorFilterImageFilter.cpp
+++ b/src/effects/SkColorFilterImageFilter.cpp
@@ -82,7 +82,8 @@ SkColorFilterImageFilter::SkColorFilterImageFilter(SkColorFilter* cf,
     SkSafeRef(cf);
 }
 
-SkColorFilterImageFilter::SkColorFilterImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkColorFilterImageFilter::SkColorFilterImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(1, buffer) {
     fColorFilter = buffer.readColorFilter();
 }
 
diff --git a/src/effects/SkComposeImageFilter.cpp b/src/effects/SkComposeImageFilter.cpp
index 2ba2f3df27..2412d9fe87 100644
--- a/src/effects/SkComposeImageFilter.cpp
+++ b/src/effects/SkComposeImageFilter.cpp
@@ -52,5 +52,6 @@ bool SkComposeImageFilter::onFilterBounds(const SkIRect& src,
            outer->filterBounds(tmp, ctm, dst);
 }
 
-SkComposeImageFilter::SkComposeImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkComposeImageFilter::SkComposeImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(2, buffer) {
 }
diff --git a/src/effects/SkDisplacementMapEffect.cpp b/src/effects/SkDisplacementMapEffect.cpp
index 816807d04e..6e5c910edf 100644
--- a/src/effects/SkDisplacementMapEffect.cpp
+++ b/src/effects/SkDisplacementMapEffect.cpp
@@ -161,7 +161,7 @@ SkDisplacementMapEffect::~SkDisplacementMapEffect() {
 }
 
 SkDisplacementMapEffect::SkDisplacementMapEffect(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer)
+  : INHERITED(2, buffer)
 {
     fXChannelSelector = (SkDisplacementMapEffect::ChannelSelectorType) buffer.readInt();
     fYChannelSelector = (SkDisplacementMapEffect::ChannelSelectorType) buffer.readInt();
diff --git a/src/effects/SkDropShadowImageFilter.cpp b/src/effects/SkDropShadowImageFilter.cpp
index 4fc29bae83..b4d8689f33 100644
--- a/src/effects/SkDropShadowImageFilter.cpp
+++ b/src/effects/SkDropShadowImageFilter.cpp
@@ -23,8 +23,8 @@ SkDropShadowImageFilter::SkDropShadowImageFilter(SkScalar dx, SkScalar dy, SkSca
 {
 }
 
-SkDropShadowImageFilter::SkDropShadowImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer)
-{
+SkDropShadowImageFilter::SkDropShadowImageFilter(SkFlattenableReadBuffer& buffer)
+ : INHERITED(1, buffer) {
     fDx = buffer.readScalar();
     fDy = buffer.readScalar();
     fSigma = buffer.readScalar();
diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp
index 45a75ab17d..8c8798f09b 100644
--- a/src/effects/SkLightingImageFilter.cpp
+++ b/src/effects/SkLightingImageFilter.cpp
@@ -887,7 +887,7 @@ SkLightingImageFilter::~SkLightingImageFilter() {
 }
 
 SkLightingImageFilter::SkLightingImageFilter(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(1, buffer) {
     fLight = SkLight::UnflattenLight(buffer);
     fSurfaceScale = buffer.readScalar();
     buffer.validate(SkScalarIsFinite(fSurfaceScale));
@@ -903,7 +903,9 @@ void SkLightingImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
 
 SkDiffuseLightingImageFilter::SkDiffuseLightingImageFilter(SkLight* light, SkScalar surfaceScale, SkScalar kd, SkImageFilter* input, const CropRect* cropRect = NULL)
   : SkLightingImageFilter(light, surfaceScale, input, cropRect),
-    fKD(kd)
+    // According to the spec, kd can be any non-negative number :
+    // http://www.w3.org/TR/SVG/filters.html#feDiffuseLightingElement
+    fKD(kd < 0 ? 0 : kd)
 {
 }
 
@@ -911,7 +913,7 @@ SkDiffuseLightingImageFilter::SkDiffuseLightingImageFilter(SkFlattenableReadBuff
   : INHERITED(buffer)
 {
     fKD = buffer.readScalar();
-    buffer.validate(SkScalarIsFinite(fKD));
+    buffer.validate(SkScalarIsFinite(fKD) && (fKD >= 0));
 }
 
 void SkDiffuseLightingImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
@@ -985,7 +987,9 @@ bool SkDiffuseLightingImageFilter::asNewEffect(GrEffectRef** effect, GrTexture*
 
 SkSpecularLightingImageFilter::SkSpecularLightingImageFilter(SkLight* light, SkScalar surfaceScale, SkScalar ks, SkScalar shininess, SkImageFilter* input, const CropRect* cropRect)
   : SkLightingImageFilter(light, surfaceScale, input, cropRect),
-    fKS(ks),
+    // According to the spec, ks can be any non-negative number :
+    // http://www.w3.org/TR/SVG/filters.html#feSpecularLightingElement
+    fKS(ks < 0 ? 0 : ks),
     fShininess(shininess)
 {
 }
@@ -995,7 +999,7 @@ SkSpecularLightingImageFilter::SkSpecularLightingImageFilter(SkFlattenableReadBu
 {
     fKS = buffer.readScalar();
     fShininess = buffer.readScalar();
-    buffer.validate(SkScalarIsFinite(fKS) &&
+    buffer.validate(SkScalarIsFinite(fKS) && (fKS >= 0) &&
                     SkScalarIsFinite(fShininess));
 }
 
diff --git a/src/effects/SkMagnifierImageFilter.cpp b/src/effects/SkMagnifierImageFilter.cpp
index 7ac5249284..d4120598f1 100644
--- a/src/effects/SkMagnifierImageFilter.cpp
+++ b/src/effects/SkMagnifierImageFilter.cpp
@@ -232,7 +232,7 @@ void GrMagnifierEffect::getConstantColorComponents(GrColor* color, uint32_t* val
 
 ////////////////////////////////////////////////////////////////////////////////
 SkMagnifierImageFilter::SkMagnifierImageFilter(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(1, buffer) {
     float x = buffer.readScalar();
     float y = buffer.readScalar();
     float width = buffer.readScalar();
diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp
index 6f213d7118..cef0450f8b 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -58,7 +58,7 @@ SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(
 }
 
 SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(SkFlattenableReadBuffer& buffer)
-    : INHERITED(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);
diff --git a/src/effects/SkMergeImageFilter.cpp b/src/effects/SkMergeImageFilter.cpp
index 93e2335610..a755fe68cb 100755
--- a/src/effects/SkMergeImageFilter.cpp
+++ b/src/effects/SkMergeImageFilter.cpp
@@ -11,6 +11,9 @@
 #include "SkFlattenableBuffers.h"
 #include "SkValidationUtils.h"
 
+// Use 65535 as an arbitrary large number of inputs that this image filter should never overflow
+static const int kMaxInputs = 65535;
+
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkMergeImageFilter::initAllocModes() {
@@ -53,6 +56,7 @@ SkMergeImageFilter::SkMergeImageFilter(SkImageFilter* first, SkImageFilter* seco
 SkMergeImageFilter::SkMergeImageFilter(SkImageFilter* filters[], int count,
                                        const SkXfermode::Mode modes[],
                                        const CropRect* cropRect) : INHERITED(count, filters, cropRect) {
+    SkASSERT(count <= kMaxInputs);
     this->initModes(modes);
 }
 
@@ -156,7 +160,8 @@ void SkMergeImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
     }
 }
 
-SkMergeImageFilter::SkMergeImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkMergeImageFilter::SkMergeImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(kMaxInputs, buffer) {
     bool hasModes = buffer.readBool();
     if (hasModes) {
         this->initAllocModes();
diff --git a/src/effects/SkMorphologyImageFilter.cpp b/src/effects/SkMorphologyImageFilter.cpp
index c9855c9086..3f2f98557a 100644
--- a/src/effects/SkMorphologyImageFilter.cpp
+++ b/src/effects/SkMorphologyImageFilter.cpp
@@ -21,7 +21,7 @@
 #endif
 
 SkMorphologyImageFilter::SkMorphologyImageFilter(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(1, buffer) {
     fRadius.fWidth = buffer.readInt();
     fRadius.fHeight = buffer.readInt();
     buffer.validate((fRadius.fWidth >= 0) &&
diff --git a/src/effects/SkOffsetImageFilter.cpp b/src/effects/SkOffsetImageFilter.cpp
index ecaf5e6b62..aefbcba8ad 100644
--- a/src/effects/SkOffsetImageFilter.cpp
+++ b/src/effects/SkOffsetImageFilter.cpp
@@ -79,7 +79,8 @@ SkOffsetImageFilter::SkOffsetImageFilter(SkScalar dx, SkScalar dy, SkImageFilter
     fOffset.set(dx, dy);
 }
 
-SkOffsetImageFilter::SkOffsetImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkOffsetImageFilter::SkOffsetImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(1, buffer) {
     buffer.readPoint(&fOffset);
     buffer.validate(SkScalarIsFinite(fOffset.fX) &&
                     SkScalarIsFinite(fOffset.fY));
diff --git a/src/effects/SkPerlinNoiseShader.cpp b/src/effects/SkPerlinNoiseShader.cpp
index 51675ad2bb..26771c5db7 100644
--- a/src/effects/SkPerlinNoiseShader.cpp
+++ b/src/effects/SkPerlinNoiseShader.cpp
@@ -51,6 +51,11 @@ inline SkScalar smoothCurve(SkScalar t) {
     return SkScalarMul(SkScalarSquare(t), SK_Scalar3 - 2 * t);
 }
 
+bool perlin_noise_type_is_valid(SkPerlinNoiseShader::Type type) {
+    return (SkPerlinNoiseShader::kFractalNoise_Type == type) ||
+           (SkPerlinNoiseShader::kTurbulence_Type == type);
+}
+
 } // end namespace
 
 struct SkPerlinNoiseShader::StitchData {
@@ -279,7 +284,7 @@ SkPerlinNoiseShader::SkPerlinNoiseShader(SkPerlinNoiseShader::Type type,
   : fType(type)
   , fBaseFrequencyX(baseFrequencyX)
   , fBaseFrequencyY(baseFrequencyY)
-  , fNumOctaves(numOctaves & 0xFF /*[0,255] octaves allowed*/)
+  , fNumOctaves(numOctaves > 255 ? 255 : numOctaves/*[0,255] octaves allowed*/)
   , fSeed(seed)
   , fStitchTiles((tileSize != NULL) && !tileSize->isEmpty())
   , fPaintingData(NULL)
@@ -301,6 +306,8 @@ SkPerlinNoiseShader::SkPerlinNoiseShader(SkFlattenableReadBuffer& buffer) :
     fTileSize.fHeight = buffer.readInt();
     setTileSize(fTileSize);
     fMatrix.reset();
+    buffer.validate(perlin_noise_type_is_valid(fType) &&
+                    (fNumOctaves >= 0) && (fNumOctaves <= 255));
 }
 
 SkPerlinNoiseShader::~SkPerlinNoiseShader() {
diff --git a/src/effects/SkRectShaderImageFilter.cpp b/src/effects/SkRectShaderImageFilter.cpp
index 81aa91ca71..ab38fc4879 100644
--- a/src/effects/SkRectShaderImageFilter.cpp
+++ b/src/effects/SkRectShaderImageFilter.cpp
@@ -35,7 +35,7 @@ SkRectShaderImageFilter::SkRectShaderImageFilter(SkShader* s, const CropRect* cr
 }
 
 SkRectShaderImageFilter::SkRectShaderImageFilter(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(1, buffer) {
     fShader = buffer.readShader();
 }
 
diff --git a/src/effects/SkTestImageFilters.cpp b/src/effects/SkTestImageFilters.cpp
index 289113133c..cc6d1adc43 100755
--- a/src/effects/SkTestImageFilters.cpp
+++ b/src/effects/SkTestImageFilters.cpp
@@ -76,7 +76,8 @@ void SkDownSampleImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {
     buffer.writeScalar(fScale);
 }
 
-SkDownSampleImageFilter::SkDownSampleImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkDownSampleImageFilter::SkDownSampleImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(1, buffer) {
     fScale = buffer.readScalar();
     buffer.validate(SkScalarIsFinite(fScale));
 }
diff --git a/src/effects/SkTileImageFilter.cpp b/src/effects/SkTileImageFilter.cpp
index 8cc096cf37..ccca4ff74c 100644
--- a/src/effects/SkTileImageFilter.cpp
+++ b/src/effects/SkTileImageFilter.cpp
@@ -54,7 +54,8 @@ bool SkTileImageFilter::onFilterImage(Proxy* proxy, const SkBitmap& src, const S
     return true;
 }
 
-SkTileImageFilter::SkTileImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
+SkTileImageFilter::SkTileImageFilter(SkFlattenableReadBuffer& buffer)
+  : INHERITED(1, buffer) {
     buffer.readRect(&fSrcRect);
     buffer.readRect(&fDstRect);
     buffer.validate(SkIsValidRect(fSrcRect) && SkIsValidRect(fDstRect));
diff --git a/src/effects/SkXfermodeImageFilter.cpp b/src/effects/SkXfermodeImageFilter.cpp
index e8fa0d2787..620bde9fdb 100644
--- a/src/effects/SkXfermodeImageFilter.cpp
+++ b/src/effects/SkXfermodeImageFilter.cpp
@@ -33,7 +33,7 @@ SkXfermodeImageFilter::~SkXfermodeImageFilter() {
 }
 
 SkXfermodeImageFilter::SkXfermodeImageFilter(SkFlattenableReadBuffer& buffer)
-  : INHERITED(buffer) {
+  : INHERITED(2, buffer) {
     fMode = buffer.readXfermode();
 }