commit 41a930f85bc3988cfbb47a902b03a9830f8149bb
author Mike Reed <reed@google.com> Wed Jul 26 17:33:44 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Jul 27 12:15:03 2017 +0000
tree d912d3e1f69ce84cce68199e172e9538d97f49c9
parent 6cd51b51d6603a3100b147c45f38697f2f199fc6

Revert "Revert "Fix SkPathRef deserialization malloc crash""

This reverts commit a4ce4b1f6bef22e7ca5c7a952197fc2bc70923fc.

Fix SkPathRef deserialization malloc crash

If the path says it has more points/verbs/etc than the buffer could
be holding, then resetToSize could try to allocate something huge
and crash.

Bug: skia:
Change-Id: I23b8870e9f74386aca89fb8f9a60d3b452044094
Reviewed-on: https://skia-review.googlesource.com/26805
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>

src/core/SkPath.cpp

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 2be3251..e304b4d 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -8,6 +8,7 @@
 #include <cmath>
 #include "SkBuffer.h"
 #include "SkCubicClipper.h"
+#include "SkData.h"
 #include "SkGeometry.h"
 #include "SkMath.h"
 #include "SkPathPriv.h"
@@ -2061,6 +2062,13 @@
     return buffer.pos();
 }
 
+sk_sp<SkData> SkPath::serialize() const {
+    size_t size = this->writeToMemory(nullptr);
+    sk_sp<SkData> data = SkData::MakeUninitialized(size);
+    this->writeToMemory(data->writable_data());
+    return data;
+}
+
 size_t SkPath::readFromMemory(const void* storage, size_t length) {
     SkRBuffer buffer(storage, length);
 

src/core/SkPathRef.cpp

diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index 3338225..abe1f90 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -243,29 +243,42 @@
     unsigned rrectOrOvalStartIdx = (packed >> kRRectOrOvalStartIdx_SerializationShift) & 0x7;
 
     int32_t verbCount, pointCount, conicCount;
-    ptrdiff_t maxPtrDiff = std::numeric_limits<ptrdiff_t>::max();
     if (!buffer->readU32(&(ref->fGenerationID)) ||
-        !buffer->readS32(&verbCount) ||
-        verbCount < 0 ||
-        static_cast<uint32_t>(verbCount) > maxPtrDiff/sizeof(uint8_t) ||
-        !buffer->readS32(&pointCount) ||
-        pointCount < 0 ||
-        static_cast<uint32_t>(pointCount) > maxPtrDiff/sizeof(SkPoint) ||
-        sizeof(uint8_t) * verbCount + sizeof(SkPoint) * pointCount >
-            static_cast<size_t>(maxPtrDiff) ||
-        !buffer->readS32(&conicCount) ||
-        conicCount < 0) {
+        !buffer->readS32(&verbCount)            || (verbCount  < 0) ||
+        !buffer->readS32(&pointCount)           || (pointCount < 0) ||
+        !buffer->readS32(&conicCount)           || (conicCount < 0))
+    {
         return nullptr;
     }
 
+    uint64_t pointSize64 = sk_64_mul(pointCount, sizeof(SkPoint));
+    uint64_t conicSize64 = sk_64_mul(conicCount, sizeof(SkScalar));
+    if (!SkTFitsIn<size_t>(pointSize64) || !SkTFitsIn<size_t>(conicSize64)) {
+        return nullptr;
+    }
+
+    size_t verbSize = verbCount * sizeof(uint8_t);
+    size_t pointSize = SkToSizeT(pointSize64);
+    size_t conicSize = SkToSizeT(conicSize64);
+
+    {
+        uint64_t requiredBufferSize = sizeof(SkRect);
+        requiredBufferSize += verbSize;
+        requiredBufferSize += pointSize;
+        requiredBufferSize += conicSize;
+        if (buffer->available() < requiredBufferSize) {
+            return nullptr;
+        }
+    }
+
     ref->resetToSize(verbCount, pointCount, conicCount);
-    SkASSERT(verbCount == ref->countVerbs());
+    SkASSERT(verbCount  == ref->countVerbs());
     SkASSERT(pointCount == ref->countPoints());
     SkASSERT(conicCount == ref->fConicWeights.count());
 
-    if (!buffer->read(ref->verbsMemWritable(), verbCount * sizeof(uint8_t)) ||
-        !buffer->read(ref->fPoints, pointCount * sizeof(SkPoint)) ||
-        !buffer->read(ref->fConicWeights.begin(), conicCount * sizeof(SkScalar)) ||
+    if (!buffer->read(ref->verbsMemWritable(), verbSize) ||
+        !buffer->read(ref->fPoints, pointSize) ||
+        !buffer->read(ref->fConicWeights.begin(), conicSize) ||
         !buffer->read(&ref->fBounds, sizeof(SkRect))) {
         return nullptr;
     }