[Backport] Simplify / fix SkBitmap::ReadRawPixels()

We no longer need to look at the field snugRB except to check for the
simple no-pixels case.  This is good, because our snugRB <= ramRB check
is actually too weak, and is the source of this linked Chromium issue.

BUG=chromium:765858

Instead of doing complicated checks against that stored snugRB and the
computed ramRB, we now just ignore snugRB.  We know the images written
by write_row_bytes() will be snug, so we can just look at width, height,
and color type to figure out exactly how many bytes we should be
reading.

Then it becomes the call to readByteArray()'s responsibility to make
sure that we have an array there of exactly that many bytes to read.
We've just got to make sure we check for its failure.

Reviewed-on: https://skia-review.googlesource.com/50800
(CVE-2017-15418)

Change-Id: I39d04a7205dc7c4f59838fc53005ebb74181adbd
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

1 files changed, 9 insertions, 22 deletions

diff --git a/chromium/third_party/skia/src/core/SkBitmap.cpp b/chromium/third_party/skia/src/core/SkBitmap.cpp
index 005e2157fe5..5734567f212 100644
--- a/chromium/third_party/skia/src/core/SkBitmap.cpp
+++ b/chromium/third_party/skia/src/core/SkBitmap.cpp
@@ -656,9 +656,8 @@ void SkBitmap::WriteRawPixels(SkWriteBuffer* buffer, const SkBitmap& bitmap) {
 }
 
 bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
-    const size_t snugRB = buffer->readUInt();
-    if (0 == snugRB) {  // no pixels
-        return false;
+    if (0 == buffer->readUInt()) {
+        return false;  // no pixels
     }
 
     SkImageInfo info;
@@ -669,34 +668,22 @@ bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
     }
 
     // If there was an error reading "info" or if it is bogus,
-    // don't use it to compute minRowBytes()
+    // don't use it to compute minRowBytes().
     if (!buffer->validate(SkColorTypeValidateAlphaType(info.colorType(),
                                                        info.alphaType()))) {
         return false;
     }
 
-    const size_t ramRB = info.minRowBytes();
-    const int height = SkMax32(info.height(), 0);
-    const uint64_t snugSize = sk_64_mul(snugRB, height);
-    const uint64_t ramSize = sk_64_mul(ramRB, height);
-    static const uint64_t max_size_t = (size_t)(-1);
-    if (!buffer->validate((snugSize <= ramSize) && (ramSize <= max_size_t))) {
+    // write_raw_pixels() always writes snug buffers with rowBytes == minRowBytes().
+    size_t bytes = info.getSafeSize(info.minRowBytes());
+    if (!buffer->validate(bytes != 0)) {
         return false;
     }
 
-    sk_sp<SkData> data(SkData::MakeUninitialized(SkToSizeT(ramSize)));
+    sk_sp<SkData> data(SkData::MakeUninitialized(bytes));
     unsigned char* dst = (unsigned char*)data->writable_data();
-    buffer->readByteArray(dst, SkToSizeT(snugSize));
-
-    if (snugSize != ramSize) {
-        const unsigned char* srcRow = dst + snugRB * (height - 1);
-        unsigned char* dstRow = dst + ramRB * (height - 1);
-        for (int y = height - 1; y >= 1; --y) {
-            memmove(dstRow, srcRow, snugRB);
-            srcRow -= snugRB;
-            dstRow -= ramRB;
-        }
-        SkASSERT(srcRow == dstRow); // first row does not need to be moved
+    if (!buffer->readByteArray(dst, bytes)) {
+        return false;
     }
 
     if (buffer->readBool()) {