diff options
author | 2016-09-23 14:23:22 -0400 | |
---|---|---|
committer | 2016-09-23 19:44:57 +0000 | |
commit | 49da334086c4a2c0bc4cb99e97965600dcb72f73 (patch) | |
tree | e901a94815caa32d4e70f40f25621681cbb6f174 /src | |
parent | aa840647fc7f057715bce62489b96c4299385135 (diff) |
Add validation of RRects to SkValidatingReadBuffer
This comes from the Skia fuzzer where it is inverting the RRect's rect which causes trouble down the line.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2591
Change-Id: I5c34105a47369492d2df99d39a4e29116060ad37
Reviewed-on: https://skia-review.googlesource.com/2591
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/core/SkRRect.cpp | 69 | ||||
-rw-r--r-- | src/core/SkValidatingReadBuffer.cpp | 5 |
2 files changed, 43 insertions, 31 deletions
diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index 2195dcfb13..1188989cdb 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -46,7 +46,7 @@ void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) { // TODO: assert that all the x&y radii are already W/2 & H/2 } - SkDEBUGCODE(this->validate();) + SkASSERT(this->isValid()); } void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad, @@ -108,7 +108,7 @@ void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad fRadii[kLowerRight_Corner].set(rightRad, bottomRad); fRadii[kLowerLeft_Corner].set(leftRad, bottomRad); - SkDEBUGCODE(this->validate();) + SkASSERT(this->isValid()); } // These parameters intentionally double. Apropos crbug.com/463920, if one of the @@ -193,7 +193,7 @@ void SkRRect::scaleRadii() { // At this point we're either oval, simple, or complex (not empty or rect). this->computeType(); - SkDEBUGCODE(this->validate();) + SkASSERT(this->isValid()); } // This method determines if a point known to be inside the RRect's bounds is @@ -290,7 +290,7 @@ static bool radii_are_nine_patch(const SkVector radii[4]) { void SkRRect::computeType() { struct Validator { Validator(const SkRRect* r) : fR(r) {} - ~Validator() { SkDEBUGCODE(fR->validate();) } + ~Validator() { SkASSERT(fR->isValid()); } const SkRRect* fR; } autoValidate(this); @@ -386,7 +386,7 @@ bool SkRRect::transform(const SkMatrix& matrix, SkRRect* dst) const { dst->fRadii[i].fX = SkScalarHalf(newRect.width()); dst->fRadii[i].fY = SkScalarHalf(newRect.height()); } - SkDEBUGCODE(dst->validate();) + SkASSERT(dst->isValid()); return true; } @@ -503,18 +503,14 @@ void SkRRect::dump(bool asHex) const { /////////////////////////////////////////////////////////////////////////////// -#ifdef SK_DEBUG /** * We need all combinations of predicates to be true to have a "safe" radius value. */ -static void validate_radius_check_predicates(SkScalar rad, SkScalar min, SkScalar max) { - SkASSERT(min <= max); - SkASSERT(rad <= max - min); - SkASSERT(min + rad <= max); - SkASSERT(max - rad >= min); +static bool are_radius_check_predicates_valid(SkScalar rad, SkScalar min, SkScalar max) { + return (min <= max) && (rad <= max - min) && (min + rad <= max) && (max - rad >= min); } -void SkRRect::validate() const { +bool SkRRect::isValid() const { bool allRadiiZero = (0 == fRadii[0].fX && 0 == fRadii[0].fY); bool allCornersSquare = (0 == fRadii[0].fX || 0 == fRadii[0].fY); bool allRadiiSame = true; @@ -536,43 +532,54 @@ void SkRRect::validate() const { switch (fType) { case kEmpty_Type: - SkASSERT(fRect.isEmpty()); - SkASSERT(allRadiiZero && allRadiiSame && allCornersSquare); + if (!fRect.isEmpty() || !allRadiiZero || !allRadiiSame || !allCornersSquare) { + return false; + } break; case kRect_Type: - SkASSERT(!fRect.isEmpty()); - SkASSERT(allRadiiZero && allRadiiSame && allCornersSquare); + if (fRect.isEmpty() || !allRadiiZero || !allRadiiSame || !allCornersSquare) { + return false; + } break; case kOval_Type: - SkASSERT(!fRect.isEmpty()); - SkASSERT(!allRadiiZero && allRadiiSame && !allCornersSquare); + if (fRect.isEmpty() || allRadiiZero || !allRadiiSame || allCornersSquare) { + return false; + } for (int i = 0; i < 4; ++i) { - SkASSERT(SkScalarNearlyEqual(fRadii[i].fX, SkScalarHalf(fRect.width()))); - SkASSERT(SkScalarNearlyEqual(fRadii[i].fY, SkScalarHalf(fRect.height()))); + if (!SkScalarNearlyEqual(fRadii[i].fX, SkScalarHalf(fRect.width())) || + !SkScalarNearlyEqual(fRadii[i].fY, SkScalarHalf(fRect.height()))) { + return false; + } } break; case kSimple_Type: - SkASSERT(!fRect.isEmpty()); - SkASSERT(!allRadiiZero && allRadiiSame && !allCornersSquare); + if (fRect.isEmpty() || allRadiiZero || !allRadiiSame || allCornersSquare) { + return false; + } break; case kNinePatch_Type: - SkASSERT(!fRect.isEmpty()); - SkASSERT(!allRadiiZero && !allRadiiSame && !allCornersSquare); - SkASSERT(patchesOfNine); + if (fRect.isEmpty() || allRadiiZero || allRadiiSame || allCornersSquare || + !patchesOfNine) { + return false; + } break; case kComplex_Type: - SkASSERT(!fRect.isEmpty()); - SkASSERT(!allRadiiZero && !allRadiiSame && !allCornersSquare); - SkASSERT(!patchesOfNine); + if (fRect.isEmpty() || allRadiiZero || allRadiiSame || allCornersSquare || + patchesOfNine) { + return false; + } break; } for (int i = 0; i < 4; ++i) { - validate_radius_check_predicates(fRadii[i].fX, fRect.fLeft, fRect.fRight); - validate_radius_check_predicates(fRadii[i].fY, fRect.fTop, fRect.fBottom); + if (!are_radius_check_predicates_valid(fRadii[i].fX, fRect.fLeft, fRect.fRight) || + !are_radius_check_predicates_valid(fRadii[i].fY, fRect.fTop, fRect.fBottom)) { + return false; + } } + + return true; } -#endif // SK_DEBUG /////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp index 326d0cf575..1566af9488 100644 --- a/src/core/SkValidatingReadBuffer.cpp +++ b/src/core/SkValidatingReadBuffer.cpp @@ -144,6 +144,11 @@ void SkValidatingReadBuffer::readRRect(SkRRect* rrect) { const void* ptr = this->skip(sizeof(SkRRect)); if (!fError) { memcpy(rrect, ptr, sizeof(SkRRect)); + this->validate(rrect->isValid()); + } + + if (fError) { + rrect->setEmpty(); } } |