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
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 2fae75a..429d092 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -1622,19 +1622,21 @@
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 9bf3925..cca22bb 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -53,20 +53,27 @@
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 5148e13..87ff046 100644
--- a/src/effects/SkBicubicImageFilter.cpp
+++ b/src/effects/SkBicubicImageFilter.cpp
@@ -40,7 +40,8 @@
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 854df9d..72f51f8 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 262733a..9bceda7 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 fdcc7ff..2042c12 100755
--- a/src/effects/SkColorFilterImageFilter.cpp
+++ b/src/effects/SkColorFilterImageFilter.cpp
@@ -82,7 +82,8 @@
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 2ba2f3d..2412d9f 100644
--- a/src/effects/SkComposeImageFilter.cpp
+++ b/src/effects/SkComposeImageFilter.cpp
@@ -52,5 +52,6 @@
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 816807d..6e5c910 100644
--- a/src/effects/SkDisplacementMapEffect.cpp
+++ b/src/effects/SkDisplacementMapEffect.cpp
@@ -161,7 +161,7 @@
}
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 4fc29ba..b4d8689 100644
--- a/src/effects/SkDropShadowImageFilter.cpp
+++ b/src/effects/SkDropShadowImageFilter.cpp
@@ -23,8 +23,8 @@
{
}
-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 45a75ab..8c8798f 100644
--- a/src/effects/SkLightingImageFilter.cpp
+++ b/src/effects/SkLightingImageFilter.cpp
@@ -887,7 +887,7 @@
}
SkLightingImageFilter::SkLightingImageFilter(SkFlattenableReadBuffer& buffer)
- : INHERITED(buffer) {
+ : INHERITED(1, buffer) {
fLight = SkLight::UnflattenLight(buffer);
fSurfaceScale = buffer.readScalar();
buffer.validate(SkScalarIsFinite(fSurfaceScale));
@@ -903,7 +903,9 @@
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 @@
: 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 @@
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 @@
{
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 7ac5249..d412059 100644
--- a/src/effects/SkMagnifierImageFilter.cpp
+++ b/src/effects/SkMagnifierImageFilter.cpp
@@ -232,7 +232,7 @@
////////////////////////////////////////////////////////////////////////////////
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 6f213d7..cef0450 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -58,7 +58,7 @@
}
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 93e2335..a755fe6 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* filters[], int count,
const SkXfermode::Mode modes[],
const CropRect* cropRect) : INHERITED(count, filters, cropRect) {
+ SkASSERT(count <= kMaxInputs);
this->initModes(modes);
}
@@ -156,7 +160,8 @@
}
}
-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 c9855c9..3f2f985 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 ecaf5e6..aefbcba 100644
--- a/src/effects/SkOffsetImageFilter.cpp
+++ b/src/effects/SkOffsetImageFilter.cpp
@@ -79,7 +79,8 @@
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 51675ad..26771c5 100644
--- a/src/effects/SkPerlinNoiseShader.cpp
+++ b/src/effects/SkPerlinNoiseShader.cpp
@@ -51,6 +51,11 @@
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 @@
: 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 @@
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 81aa91c..ab38fc4 100644
--- a/src/effects/SkRectShaderImageFilter.cpp
+++ b/src/effects/SkRectShaderImageFilter.cpp
@@ -35,7 +35,7 @@
}
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 28911313..cc6d1ad 100755
--- a/src/effects/SkTestImageFilters.cpp
+++ b/src/effects/SkTestImageFilters.cpp
@@ -76,7 +76,8 @@
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 8cc096c..ccca4ff 100644
--- a/src/effects/SkTileImageFilter.cpp
+++ b/src/effects/SkTileImageFilter.cpp
@@ -54,7 +54,8 @@
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 e8fa0d2..620bde9 100644
--- a/src/effects/SkXfermodeImageFilter.cpp
+++ b/src/effects/SkXfermodeImageFilter.cpp
@@ -33,7 +33,7 @@
}
SkXfermodeImageFilter::SkXfermodeImageFilter(SkFlattenableReadBuffer& buffer)
- : INHERITED(buffer) {
+ : INHERITED(2, buffer) {
fMode = buffer.readXfermode();
}