Fixed bad bitmap size crashes
There were 2 issues :
1 ) If the size of an SkBitmap's underlying SkPixelRef's alocated memory is too small to fit the bitmap, then the deserialization will now check this and set an error appropriately.
2 ) If a device fails to allocate its pixels, the device will be deleted and NULL will be returned to avoid attempting to draw on a bad device.
BUG=
R=senorblanco@chromium.org, reed@google.com, sugoi@google.com, halcanary@google.com, mtklein@google.com
Author: sugoi@chromium.org
Review URL: https://codereview.chromium.org/92793002
git-svn-id: http://skia.googlecode.com/svn/trunk@12484 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 9d4aa87..ad840c4 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -1560,6 +1560,7 @@
SkIsValidConfig(config) && validate_alphaType(config, alphaType));
this->setConfig(config, width, height, rowBytes, alphaType);
+ buffer.validate(fRowBytes >= (fWidth * fBytesPerPixel));
int reftype = buffer.readInt();
if (buffer.validate((SERIALIZE_PIXELTYPE_REF_DATA == reftype) ||
@@ -1568,6 +1569,10 @@
case SERIALIZE_PIXELTYPE_REF_DATA: {
size_t offset = buffer.readUInt();
SkPixelRef* pr = buffer.readPixelRef();
+ if (!buffer.validate((NULL == pr) ||
+ (pr->getAllocatedSizeInBytes() >= (offset + this->getSafeSize())))) {
+ offset = 0;
+ }
SkSafeUnref(this->setPixelRef(pr, offset));
break;
}
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 369f10c..2d7e413 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -29,7 +29,10 @@
SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque) {
fBitmap.setConfig(config, width, height, 0, isOpaque ?
kOpaque_SkAlphaType : kPremul_SkAlphaType);
- fBitmap.allocPixels();
+ if (!fBitmap.allocPixels()) {
+ fBitmap.setConfig(config, 0, 0, 0, isOpaque ?
+ kOpaque_SkAlphaType : kPremul_SkAlphaType);
+ }
if (!isOpaque) {
fBitmap.eraseColor(SK_ColorTRANSPARENT);
}
@@ -41,7 +44,10 @@
fBitmap.setConfig(config, width, height, 0, isOpaque ?
kOpaque_SkAlphaType : kPremul_SkAlphaType);
- fBitmap.allocPixels();
+ if (!fBitmap.allocPixels()) {
+ fBitmap.setConfig(config, 0, 0, 0, isOpaque ?
+ kOpaque_SkAlphaType : kPremul_SkAlphaType);
+ }
if (!isOpaque) {
fBitmap.eraseColor(SK_ColorTRANSPARENT);
}
@@ -61,8 +67,14 @@
int width, int height,
bool isOpaque,
Usage usage) {
- return SkNEW_ARGS(SkBitmapDevice,(config, width, height, isOpaque,
- this->getDeviceProperties()));
+ SkBitmapDevice* device = SkNEW_ARGS(SkBitmapDevice,(config, width, height,
+ isOpaque, this->getDeviceProperties()));
+ // Check if allocation failed and delete device if it did fail
+ if ((device->width() != width) || (device->height() != height)) {
+ SkDELETE(device);
+ device = NULL;
+ }
+ return device;
}
void SkBitmapDevice::lockPixels() {
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 972474c..068513b 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -247,6 +247,10 @@
return NULL;
}
+size_t SkPixelRef::getAllocatedSizeInBytes() const {
+ return 0;
+}
+
///////////////////////////////////////////////////////////////////////////////
#ifdef SK_BUILD_FOR_ANDROID
diff --git a/src/effects/SkBlurImageFilter.cpp b/src/effects/SkBlurImageFilter.cpp
index 9bceda7..2795f3a 100644
--- a/src/effects/SkBlurImageFilter.cpp
+++ b/src/effects/SkBlurImageFilter.cpp
@@ -159,6 +159,10 @@
dst->setConfig(src.config(), srcBounds.width(), srcBounds.height());
dst->getBounds(&dstBounds);
dst->allocPixels();
+ if (!dst->getPixels()) {
+ return false;
+ }
+
int kernelSizeX, kernelSizeX3, lowOffsetX, highOffsetX;
int kernelSizeY, kernelSizeY3, lowOffsetY, highOffsetY;
getBox3Params(fSigma.width(), &kernelSizeX, &kernelSizeX3, &lowOffsetX, &highOffsetX);
diff --git a/src/effects/SkColorFilterImageFilter.cpp b/src/effects/SkColorFilterImageFilter.cpp
index 2042c12..8b7b390 100755
--- a/src/effects/SkColorFilterImageFilter.cpp
+++ b/src/effects/SkColorFilterImageFilter.cpp
@@ -113,6 +113,9 @@
}
SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+ if (NULL == device.get()) {
+ return false;
+ }
SkCanvas canvas(device.get());
SkPaint paint;
diff --git a/src/effects/SkDropShadowImageFilter.cpp b/src/effects/SkDropShadowImageFilter.cpp
index 5be633e..24a910d 100644
--- a/src/effects/SkDropShadowImageFilter.cpp
+++ b/src/effects/SkDropShadowImageFilter.cpp
@@ -70,6 +70,9 @@
}
SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+ if (NULL == device.get()) {
+ return false;
+ }
SkCanvas canvas(device.get());
SkAutoTUnref<SkImageFilter> blurFilter(new SkBlurImageFilter(fSigmaX, fSigmaY));
diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp
index 8c8798f..4e3cee0 100644
--- a/src/effects/SkLightingImageFilter.cpp
+++ b/src/effects/SkLightingImageFilter.cpp
@@ -814,6 +814,7 @@
case SkLight::kSpot_LightType: return SkNEW_ARGS(SkSpotLight, (buffer));
default:
SkDEBUGFAIL("Unknown LightType.");
+ buffer.validate(false);
return NULL;
}
}
@@ -952,6 +953,9 @@
dst->setConfig(src.config(), bounds.width(), bounds.height());
dst->allocPixels();
+ if (!dst->getPixels()) {
+ return false;
+ }
SkAutoTUnref<SkLight> transformedLight(light()->transform(ctm));
@@ -1040,6 +1044,9 @@
dst->setConfig(src.config(), bounds.width(), bounds.height());
dst->allocPixels();
+ if (!dst->getPixels()) {
+ return false;
+ }
SpecularLightingType lightingType(fKS, fShininess);
SkAutoTUnref<SkLight> transformedLight(light()->transform(ctm));
diff --git a/src/effects/SkMagnifierImageFilter.cpp b/src/effects/SkMagnifierImageFilter.cpp
index d412059..e6f3984 100644
--- a/src/effects/SkMagnifierImageFilter.cpp
+++ b/src/effects/SkMagnifierImageFilter.cpp
@@ -240,7 +240,9 @@
fSrcRect = SkRect::MakeXYWH(x, y, width, height);
fInset = buffer.readScalar();
- buffer.validate(SkIsValidRect(fSrcRect) && SkScalarIsFinite(fInset));
+ buffer.validate(SkScalarIsFinite(fInset) && SkIsValidRect(fSrcRect) &&
+ // Negative numbers in src rect are not supported
+ (fSrcRect.fLeft >= 0) && (fSrcRect.fTop >= 0));
}
// FIXME: implement single-input semantics
@@ -283,7 +285,9 @@
SkASSERT(fSrcRect.width() < src.width());
SkASSERT(fSrcRect.height() < src.height());
- if (src.config() != SkBitmap::kARGB_8888_Config) {
+ if ((src.config() != SkBitmap::kARGB_8888_Config) ||
+ (fSrcRect.width() >= src.width()) ||
+ (fSrcRect.height() >= src.height())) {
return false;
}
@@ -293,13 +297,17 @@
return false;
}
+ dst->setConfig(src.config(), src.width(), src.height());
+ dst->allocPixels();
+ if (!dst->getPixels()) {
+ return false;
+ }
+
SkScalar inv_inset = fInset > 0 ? SkScalarInvert(fInset) : SK_Scalar1;
SkScalar inv_x_zoom = fSrcRect.width() / src.width();
SkScalar inv_y_zoom = fSrcRect.height() / src.height();
- dst->setConfig(src.config(), src.width(), src.height());
- dst->allocPixels();
SkColor* sptr = src.getAddr32(0, 0);
SkColor* dptr = dst->getAddr32(0, 0);
int width = src.width(), height = src.height();
@@ -332,8 +340,8 @@
SkScalar y_interp = SkScalarMul(weight, (fSrcRect.y() + y * inv_y_zoom)) +
(SK_Scalar1 - weight) * y;
- int x_val = SkMin32(SkScalarFloorToInt(x_interp), width - 1);
- int y_val = SkMin32(SkScalarFloorToInt(y_interp), height - 1);
+ int x_val = SkPin32(SkScalarFloorToInt(x_interp), 0, width - 1);
+ int y_val = SkPin32(SkScalarFloorToInt(y_interp), 0, height - 1);
*dptr = sptr[y_val * width + x_val];
dptr++;
diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp
index cef0450..3da27ce 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -279,6 +279,9 @@
result->setConfig(src.config(), bounds.width(), bounds.height());
result->allocPixels();
+ if (!result->getPixels()) {
+ return false;
+ }
SkIRect interior = SkIRect::MakeXYWH(bounds.left() + fTarget.fX,
bounds.top() + fTarget.fY,
diff --git a/src/effects/SkMorphologyImageFilter.cpp b/src/effects/SkMorphologyImageFilter.cpp
index 3f2f985..0d00c35 100644
--- a/src/effects/SkMorphologyImageFilter.cpp
+++ b/src/effects/SkMorphologyImageFilter.cpp
@@ -188,6 +188,9 @@
dst->setConfig(src.config(), bounds.width(), bounds.height());
dst->allocPixels();
+ if (!dst->getPixels()) {
+ return false;
+ }
int width = radius().width();
int height = radius().height();
@@ -247,6 +250,9 @@
dst->setConfig(src.config(), bounds.width(), bounds.height());
dst->allocPixels();
+ if (!dst->getPixels()) {
+ return false;
+ }
int width = radius().width();
int height = radius().height();
diff --git a/src/effects/SkOffsetImageFilter.cpp b/src/effects/SkOffsetImageFilter.cpp
index aefbcba..59318e3 100644
--- a/src/effects/SkOffsetImageFilter.cpp
+++ b/src/effects/SkOffsetImageFilter.cpp
@@ -48,6 +48,9 @@
}
SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+ if (NULL == device.get()) {
+ return false;
+ }
SkCanvas canvas(device);
SkPaint paint;
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
diff --git a/src/effects/SkRectShaderImageFilter.cpp b/src/effects/SkRectShaderImageFilter.cpp
index ab38fc4..5c34547 100644
--- a/src/effects/SkRectShaderImageFilter.cpp
+++ b/src/effects/SkRectShaderImageFilter.cpp
@@ -62,6 +62,9 @@
SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(),
bounds.height()));
+ if (NULL == device.get()) {
+ return false;
+ }
SkCanvas canvas(device.get());
SkPaint paint;
paint.setShader(fShader);
diff --git a/src/effects/SkTileImageFilter.cpp b/src/effects/SkTileImageFilter.cpp
index ccca4ff..7d3b72f 100644
--- a/src/effects/SkTileImageFilter.cpp
+++ b/src/effects/SkTileImageFilter.cpp
@@ -38,6 +38,9 @@
}
SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(w, h));
+ if (NULL == device.get()) {
+ return false;
+ }
SkIRect bounds;
source.getBounds(&bounds);
SkCanvas canvas(device);
diff --git a/src/effects/SkXfermodeImageFilter.cpp b/src/effects/SkXfermodeImageFilter.cpp
index 620bde9..3ab5295 100644
--- a/src/effects/SkXfermodeImageFilter.cpp
+++ b/src/effects/SkXfermodeImageFilter.cpp
@@ -72,6 +72,9 @@
foregroundOffset.fY -= bounds.top();
SkAutoTUnref<SkBaseDevice> device(proxy->createDevice(bounds.width(), bounds.height()));
+ if (NULL == device.get()) {
+ return false;
+ }
SkCanvas canvas(device);
SkPaint paint;
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
diff --git a/src/image/SkDataPixelRef.cpp b/src/image/SkDataPixelRef.cpp
index 0524243..7897bf9 100644
--- a/src/image/SkDataPixelRef.cpp
+++ b/src/image/SkDataPixelRef.cpp
@@ -27,6 +27,10 @@
// nothing to do
}
+size_t SkDataPixelRef::getAllocatedSizeInBytes() const {
+ return fData ? fData->size() : 0;
+}
+
void SkDataPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
this->INHERITED::flatten(buffer);
buffer.writeDataAsByteArray(fData);
diff --git a/src/image/SkDataPixelRef.h b/src/image/SkDataPixelRef.h
index 6b15802..50c8857 100644
--- a/src/image/SkDataPixelRef.h
+++ b/src/image/SkDataPixelRef.h
@@ -22,6 +22,7 @@
protected:
virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE;
virtual void onUnlockPixels() SK_OVERRIDE;
+ virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE;
SkDataPixelRef(SkFlattenableReadBuffer& buffer);
virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE;