Fixed a few places where uninitialized memory could have been read
Also added early exit in SkImageFilter's constructor to avoid attempting to deserialize all inputs once a bad input has been found. This avoids hanging if a filter pretends to have 1 billion inputs when that's just an error on the number of inputs read by the filter.
BUG=326206,326197,326229
R=senorblanco@chromium.org, senorblanco@google.com, reed@google.com, sugoi@google.com
Author: sugoi@chromium.org
Review URL: https://codereview.chromium.org/106943002
git-svn-id: http://skia.googlecode.com/svn/trunk@12544 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index d2a308b..7e204f2 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -1557,8 +1557,11 @@
buffer.validate((width >= 0) && (height >= 0) && (rowBytes >= 0) &&
SkIsValidConfig(config) && validate_alphaType(config, alphaType));
- this->setConfig(config, width, height, rowBytes, alphaType);
- buffer.validate(fRowBytes >= (fWidth * fBytesPerPixel));
+ bool configIsValid = this->setConfig(config, width, height, rowBytes, alphaType);
+ // Note : Using (fRowBytes >= (fWidth * fBytesPerPixel)) in the following test can create false
+ // positives if the multiplication causes an integer overflow. Use the division instead.
+ buffer.validate(configIsValid && (fBytesPerPixel > 0) &&
+ ((fRowBytes / fBytesPerPixel) >= fWidth));
int reftype = buffer.readInt();
if (buffer.validate((SERIALIZE_PIXELTYPE_REF_DATA == reftype) ||
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 2a221fb..cda635b 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -61,10 +61,14 @@
} else {
fInputs[i] = NULL;
}
+ if (!buffer.isValid()) {
+ fInputCount = i; // Do not use fInputs past that point in the destructor
+ break;
+ }
}
SkRect rect;
buffer.readRect(&rect);
- if (buffer.validate(SkIsValidRect(rect))) {
+ if (buffer.isValid() && buffer.validate(SkIsValidRect(rect))) {
uint32_t flags = buffer.readUInt();
fCropRect = CropRect(rect, flags);
}
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
index 4a8ac47..692d0dd 100644
--- a/src/core/SkValidatingReadBuffer.cpp
+++ b/src/core/SkValidatingReadBuffer.cpp
@@ -29,6 +29,10 @@
return !fError;
}
+bool SkValidatingReadBuffer::isValid() const {
+ return !fError;
+}
+
void SkValidatingReadBuffer::setMemory(const void* data, size_t size) {
this->validate(IsPtrAlign4(data) && (SkAlign4(size) == size));
if (!fError) {
diff --git a/src/core/SkValidatingReadBuffer.h b/src/core/SkValidatingReadBuffer.h
index 2bcdc43..febf0c0 100644
--- a/src/core/SkValidatingReadBuffer.h
+++ b/src/core/SkValidatingReadBuffer.h
@@ -61,6 +61,7 @@
virtual SkTypeface* readTypeface() SK_OVERRIDE;
virtual bool validate(bool isValid) SK_OVERRIDE;
+ virtual bool isValid() const SK_OVERRIDE;
private:
bool readArray(void* value, size_t size, size_t elementSize);
diff --git a/src/effects/SkColorFilters.cpp b/src/effects/SkColorFilters.cpp
index 8ef9edf..d482a09 100644
--- a/src/effects/SkColorFilters.cpp
+++ b/src/effects/SkColorFilters.cpp
@@ -101,8 +101,10 @@
SkModeColorFilter(SkFlattenableReadBuffer& buffer) {
fColor = buffer.readColor();
fMode = (SkXfermode::Mode)buffer.readUInt();
- this->updateCache();
- buffer.validate(SkIsValidMode(fMode));
+ if (buffer.isValid()) {
+ this->updateCache();
+ buffer.validate(SkIsValidMode(fMode));
+ }
}
private:
diff --git a/src/effects/SkColorMatrixFilter.cpp b/src/effects/SkColorMatrixFilter.cpp
index 5b36a8f..fc1b77b 100644
--- a/src/effects/SkColorMatrixFilter.cpp
+++ b/src/effects/SkColorMatrixFilter.cpp
@@ -308,10 +308,8 @@
SkColorMatrixFilter::SkColorMatrixFilter(SkFlattenableReadBuffer& buffer)
: INHERITED(buffer) {
SkASSERT(buffer.getArrayCount() == 20);
- buffer.readScalarArray(fMatrix.fMat, 20);
- this->initState(fMatrix.fMat);
- for (int i = 0; i < 20; ++i) {
- buffer.validate(SkScalarIsFinite(fMatrix.fMat[i]));
+ if (buffer.readScalarArray(fMatrix.fMat, 20)) {
+ this->initState(fMatrix.fMat);
}
}
diff --git a/src/effects/SkMergeImageFilter.cpp b/src/effects/SkMergeImageFilter.cpp
index b90c830..33673b0 100755
--- a/src/effects/SkMergeImageFilter.cpp
+++ b/src/effects/SkMergeImageFilter.cpp
@@ -165,9 +165,10 @@
int nbInputs = countInputs();
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]));
+ if (buffer.readByteArray(fModes, size)) {
+ for (int i = 0; i < nbInputs; ++i) {
+ buffer.validate(SkIsValidMode((SkXfermode::Mode)fModes[i]));
+ }
}
} else {
fModes = 0;
diff --git a/src/effects/SkTileImageFilter.cpp b/src/effects/SkTileImageFilter.cpp
index 7d3b72f..c5eace0 100644
--- a/src/effects/SkTileImageFilter.cpp
+++ b/src/effects/SkTileImageFilter.cpp
@@ -61,7 +61,7 @@
: INHERITED(1, buffer) {
buffer.readRect(&fSrcRect);
buffer.readRect(&fDstRect);
- buffer.validate(SkIsValidRect(fSrcRect) && SkIsValidRect(fDstRect));
+ buffer.validate(buffer.isValid() && SkIsValidRect(fSrcRect) && SkIsValidRect(fDstRect));
}
void SkTileImageFilter::flatten(SkFlattenableWriteBuffer& buffer) const {