diff options
author | Mike Reed <reed@google.com> | 2018-01-18 15:57:38 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2018-01-18 21:14:19 +0000 |
commit | e97e792c79afd01fe1d83e5d1e94918145794c54 (patch) | |
tree | 43dc60bf6ddffa55b0bd86a1a4a17f8819b21aca | |
parent | 9ca27849d8259e4b35243094bdca969612efba2f (diff) |
validate paint setters in readbuffer
Bug: skia:7425
Change-Id: I55213bc206cf5cfb8cbf4fbe8a682efd6eae59fa
Reviewed-on: https://skia-review.googlesource.com/96860
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Herb Derby <herb@google.com>
-rw-r--r-- | docs/SkPaint_Reference.bmh | 4 | ||||
-rw-r--r-- | include/core/SkPaint.h | 4 | ||||
-rw-r--r-- | src/core/SkPaint.cpp | 21 | ||||
-rw-r--r-- | src/core/SkPictureData.cpp | 4 | ||||
-rw-r--r-- | src/core/SkReadBuffer.h | 2 | ||||
-rw-r--r-- | src/core/SkSafeRange.h | 37 | ||||
-rw-r--r-- | src/pipe/SkPipeReader.cpp | 3 |
7 files changed, 64 insertions, 11 deletions
diff --git a/docs/SkPaint_Reference.bmh b/docs/SkPaint_Reference.bmh index b19feb29ed..b5b31498ea 100644 --- a/docs/SkPaint_Reference.bmh +++ b/docs/SkPaint_Reference.bmh @@ -577,7 +577,7 @@ can reconstitute the paint at a later time. # ------------------------------------------------------------------------------ -#Method void unflatten(SkReadBuffer& buffer) +#Method bool unflatten(SkReadBuffer& buffer) Populates Paint, typically from a serialized stream, created by calling flatten() at an earlier time. @@ -587,6 +587,8 @@ by the client. #Param buffer serialized data describing Paint content ## +#Return false if the buffer contained invalid data for initializing the paint. ## + # why is unflatten() public? #Bug 6172 ## diff --git a/include/core/SkPaint.h b/include/core/SkPaint.h index 94a7f407e0..f334570a00 100644 --- a/include/core/SkPaint.h +++ b/include/core/SkPaint.h @@ -177,8 +177,10 @@ public: by the client. @param buffer serialized data describing SkPaint content + @return false if the buffer contained invalid data to initialize the paint, in which case + the paint will be reset(). */ - void unflatten(SkReadBuffer& buffer); + bool unflatten(SkReadBuffer& buffer); /** Sets all SkPaint contents to their initial values. This is equivalent to replacing SkPaint with the result of SkPaint(). diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp index d659b41410..7c3edb3fe2 100644 --- a/src/core/SkPaint.cpp +++ b/src/core/SkPaint.cpp @@ -24,6 +24,7 @@ #include "SkPaintDefaults.h" #include "SkPathEffect.h" #include "SkRasterizer.h" +#include "SkSafeRange.h" #include "SkScalar.h" #include "SkScalerContext.h" #include "SkShader.h" @@ -1911,7 +1912,9 @@ void SkPaint::flatten(SkWriteBuffer& buffer) const { } } -void SkPaint::unflatten(SkReadBuffer& buffer) { +bool SkPaint::unflatten(SkReadBuffer& buffer) { + SkSafeRange safe; + this->setTextSize(buffer.readScalar()); this->setTextScaleX(buffer.readScalar()); this->setTextSkewX(buffer.readScalar()); @@ -1922,11 +1925,11 @@ void SkPaint::unflatten(SkReadBuffer& buffer) { unsigned flatFlags = unpack_paint_flags(this, buffer.readUInt()); uint32_t tmp = buffer.readUInt(); - this->setStrokeCap(static_cast<Cap>((tmp >> 24) & 0xFF)); - this->setStrokeJoin(static_cast<Join>((tmp >> 16) & 0xFF)); - this->setStyle(static_cast<Style>((tmp >> 12) & 0xF)); - this->setTextEncoding(static_cast<TextEncoding>((tmp >> 8) & 0xF)); - this->setBlendMode((SkBlendMode)(tmp & 0xFF)); + this->setStrokeCap(safe.checkLE((tmp >> 24) & 0xFF, kLast_Cap)); + this->setStrokeJoin(safe.checkLE((tmp >> 16) & 0xFF, kLast_Join)); + this->setStyle(safe.checkLE((tmp >> 12) & 0xF, kStrokeAndFill_Style)); + this->setTextEncoding(safe.checkLE((tmp >> 8) & 0xF, kGlyphID_TextEncoding)); + this->setBlendMode(safe.checkLE(tmp & 0xFF, SkBlendMode::kLastMode)); if (flatFlags & kHasTypeface_FlatFlag) { this->setTypeface(buffer.readTypeface()); @@ -1951,6 +1954,12 @@ void SkPaint::unflatten(SkReadBuffer& buffer) { this->setLooper(nullptr); this->setImageFilter(nullptr); } + + if (!buffer.validate(safe)) { + this->reset(); + return false; + } + return true; } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp index 247d16f6d2..0d729f8a75 100644 --- a/src/core/SkPictureData.cpp +++ b/src/core/SkPictureData.cpp @@ -505,7 +505,9 @@ bool SkPictureData::parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t const int count = SkToInt(size); fPaints.reset(count); for (int i = 0; i < count; ++i) { - buffer.readPaint(&fPaints[i]); + if (!buffer.readPaint(&fPaints[i])) { + return false; + } } } break; case SK_PICT_PATH_BUFFER_TAG: diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h index f4e9f6a0d8..65ec8364fe 100644 --- a/src/core/SkReadBuffer.h +++ b/src/core/SkReadBuffer.h @@ -134,7 +134,7 @@ public: void readRegion(SkRegion* region); void readPath(SkPath* path); - virtual void readPaint(SkPaint* paint) { paint->unflatten(*this); } + virtual bool readPaint(SkPaint* paint) { return paint->unflatten(*this); } SkFlattenable* readFlattenable(SkFlattenable::Type); template <typename T> sk_sp<T> readFlattenable() { diff --git a/src/core/SkSafeRange.h b/src/core/SkSafeRange.h new file mode 100644 index 0000000000..c640ffdd5c --- /dev/null +++ b/src/core/SkSafeRange.h @@ -0,0 +1,37 @@ +/* + * Copyright 2018 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkSafeRange_DEFINED +#define SkSafeRange_DEFINED + +// SkSafeRange always check that a series of operations are in-range. +// This check is sticky, so that if any one operation fails, the object will remember that and +// return false from ok(). + +class SkSafeRange { +public: + operator bool() const { return fOK; } + + bool ok() const { return fOK; } + + // checks 0 <= value <= max. + // On success, returns value + // On failure, returns 0 and sets ok() to false + template <typename T> T checkLE(uint64_t value, T max) { + SkASSERT(static_cast<int64_t>(max) >= 0); + if (value > static_cast<uint64_t>(max)) { + fOK = false; + value = 0; + } + return static_cast<T>(value); + } + +private: + bool fOK = true; +}; + +#endif diff --git a/src/pipe/SkPipeReader.cpp b/src/pipe/SkPipeReader.cpp index c2ff0bfbb6..ee48d5a23b 100644 --- a/src/pipe/SkPipeReader.cpp +++ b/src/pipe/SkPipeReader.cpp @@ -207,8 +207,9 @@ public: return factory; } - void readPaint(SkPaint* paint) override { + bool readPaint(SkPaint* paint) override { *paint = read_paint(*this); + return this->isValid(); } }; |