commit c2e9db30d393862bd3485cfe57b4ac06433f2f32
author commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Fri Dec 06 20:14:46 2013 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Fri Dec 06 20:14:46 2013 +0000
tree ad1a292701c5fa0444dda0724e41e9ec86721776
parent 7842c817e920a0e0c83be339f8d7e19b016c3373

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

src/core/SkBitmap.cpp

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) ||

src/core/SkImageFilter.cpp

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);
         }

src/core/SkValidatingReadBuffer.cpp

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) {

src/core/SkValidatingReadBuffer.h

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);