aboutsummaryrefslogtreecommitdiffhomepage
path: root/src
diff options
context:
space:
mode:
authorGravatar Mike Reed <reed@google.com>2017-07-26 17:33:44 -0400
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2017-07-27 12:15:03 +0000
commit41a930f85bc3988cfbb47a902b03a9830f8149bb (patch)
treed912d3e1f69ce84cce68199e172e9538d97f49c9 /src
parent6cd51b51d6603a3100b147c45f38697f2f199fc6 (diff)
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>
Diffstat (limited to 'src')
-rw-r--r--src/core/SkPath.cpp8
-rw-r--r--src/core/SkPathRef.cpp43
2 files changed, 36 insertions, 15 deletions
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 2be3251401..e304b4d8c5 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 @@ size_t SkPath::writeToMemory(void* storage) const {
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);
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index 3338225391..abe1f906a5 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -243,29 +243,42 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer) {
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;
}