diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-02-01 17:48:16 +0100 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-03-29 09:56:40 +0000 |
commit | 4299209a92a5e23dc9bfd4a8423f55be750797d6 (patch) | |
tree | fe76a33be0ebc385b090aa83dd848a02be033b8d | |
parent | 75d3bbed55d27195e0262b077eb9b2feabef96a1 (diff) | |
download | qtwebengine-chromium-4299209a92a5e23dc9bfd4a8423f55be750797d6.tar.gz |
[Backport] Fix for security issue 899689
Cherry-pick : change convex scan converter to be defensive
Intended for M72
Had to perform manual rebase to both SkPath.cpp and SkScan_Path.cpp as
they had diverged by the time I tried the cherry-pick from head.
Bug: 899689
Bug: skia:8606
Change-Id: Ie6c13dcd2e45d55faef4180ede299703f71b1412
Reviewed-On: https://skia-review.googlesource.com/c/175832
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-By: Mike Klein <mtklein@google.com>
Reviewed-By: Cary Clark <caryclark@google.com>
Reviewed-on: https://skia-review.googlesource.com/c/182443
Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
-rw-r--r-- | chromium/third_party/skia/include/core/SkPathRef.h | 1 | ||||
-rw-r--r-- | chromium/third_party/skia/src/core/SkPath.cpp | 17 | ||||
-rw-r--r-- | chromium/third_party/skia/src/core/SkPathPriv.h | 5 | ||||
-rw-r--r-- | chromium/third_party/skia/src/core/SkScan_Path.cpp | 57 |
4 files changed, 53 insertions, 27 deletions
diff --git a/chromium/third_party/skia/include/core/SkPathRef.h b/chromium/third_party/skia/include/core/SkPathRef.h index 24f76f150d2..0a4bab51a01 100644 --- a/chromium/third_party/skia/include/core/SkPathRef.h +++ b/chromium/third_party/skia/include/core/SkPathRef.h @@ -545,6 +545,7 @@ private: friend class PathRefTest_Private; friend class ForceIsRRect_Private; // unit test isRRect + friend class SkPathPriv; }; #endif diff --git a/chromium/third_party/skia/src/core/SkPath.cpp b/chromium/third_party/skia/src/core/SkPath.cpp index c8bd486e1c3..099db2e1472 100644 --- a/chromium/third_party/skia/src/core/SkPath.cpp +++ b/chromium/third_party/skia/src/core/SkPath.cpp @@ -1744,6 +1744,8 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const { matrix.mapPoints(ed.points(), ed.pathRef()->countPoints()); dst->fFirstDirection = SkPathPriv::kUnknown_FirstDirection; } else { + Convexity convexity = this->getConvexityOrUnknown(); + SkPathRef::CreateTransformedCopy(&dst->fPathRef, *fPathRef.get(), matrix); if (this != dst) { @@ -1752,6 +1754,21 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const { dst->fIsVolatile = fIsVolatile; } + // Due to finite/fragile float numerics, we can't assume that a convex path remains + // convex after a transformation, so mark it as unknown here. + // However, some transformations are thought to be safe: + // axis-aligned values under scale/translate. + // + // See skbug.com/8606 + // If we can land a robust convex scan-converter, we may be able to relax/remove this + // check, and keep convex paths marked as such after a general transform... + // + if (matrix.isScaleTranslate() && SkPathPriv::IsAxisAligned(*this)) { + dst->setConvexity(convexity); + } else { + dst->setConvexity(kUnknown_Convexity); + } + if (SkPathPriv::kUnknown_FirstDirection == fFirstDirection) { dst->fFirstDirection = SkPathPriv::kUnknown_FirstDirection; } else { diff --git a/chromium/third_party/skia/src/core/SkPathPriv.h b/chromium/third_party/skia/src/core/SkPathPriv.h index 029cb759dee..cfcdc4cac10 100644 --- a/chromium/third_party/skia/src/core/SkPathPriv.h +++ b/chromium/third_party/skia/src/core/SkPathPriv.h @@ -121,6 +121,11 @@ public: static const SkScalar* ConicWeightData(const SkPath& path) { return path.fPathRef->conicWeights(); } + + static bool IsAxisAligned(const SkPath& path) { + SkRect tmp; + return (path.fPathRef->fIsRRect | path.fPathRef->fIsOval) || path.isRect(&tmp); + } }; #endif diff --git a/chromium/third_party/skia/src/core/SkScan_Path.cpp b/chromium/third_party/skia/src/core/SkScan_Path.cpp index ec0fe06b66f..b5b8a004003 100644 --- a/chromium/third_party/skia/src/core/SkScan_Path.cpp +++ b/chromium/third_party/skia/src/core/SkScan_Path.cpp @@ -241,9 +241,17 @@ static bool update_edge(SkEdge* edge, int last_y) { return false; } -static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType, - SkBlitter* blitter, int start_y, int stop_y, - PrePostProc proc) { +// Unexpected conditions for which we need to return +#define ASSERT_RETURN(cond) \ + do { \ + if (!(cond)) { \ + SkASSERT(false); \ + return; \ + } \ + } while (0) + +// Needs Y to only change once (looser than convex in X) +static void walk_simple_edges(SkEdge* prevHead, SkBlitter* blitter, int start_y, int stop_y) { validate_sort(prevHead->fNext); SkEdge* leftE = prevHead->fNext; @@ -258,30 +266,29 @@ static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType, // not lining up, so we take the max. int local_top = SkMax32(leftE->fFirstY, riteE->fFirstY); #endif - SkASSERT(local_top >= start_y); + ASSERT_RETURN(local_top >= start_y); - for (;;) { + while (local_top < stop_y) { SkASSERT(leftE->fFirstY <= stop_y); SkASSERT(riteE->fFirstY <= stop_y); - if (leftE->fX > riteE->fX || (leftE->fX == riteE->fX && - leftE->fDX > riteE->fDX)) { - SkTSwap(leftE, riteE); - } - int local_bot = SkMin32(leftE->fLastY, riteE->fLastY); local_bot = SkMin32(local_bot, stop_y - 1); - SkASSERT(local_top <= local_bot); + ASSERT_RETURN(local_top <= local_bot); SkFixed left = leftE->fX; SkFixed dLeft = leftE->fDX; SkFixed rite = riteE->fX; SkFixed dRite = riteE->fDX; int count = local_bot - local_top; - SkASSERT(count >= 0); + ASSERT_RETURN(count >= 0); + if (0 == (dLeft | dRite)) { int L = SkFixedRoundToInt(left); int R = SkFixedRoundToInt(rite); + if (L > R) { + std::swap(L, R); + } if (L < R) { count += 1; blitter->blitRect(L, local_top, R - L, count); @@ -291,6 +298,9 @@ static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType, do { int L = SkFixedRoundToInt(left); int R = SkFixedRoundToInt(rite); + if (L > R) { + std::swap(L, R); + } if (L < R) { blitter->blitH(L, local_top, R - L); } @@ -305,26 +315,19 @@ static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType, if (update_edge(leftE, local_bot)) { if (currE->fFirstY >= stop_y) { - break; + return; // we're done } leftE = currE; currE = currE->fNext; + ASSERT_RETURN(leftE->fFirstY == local_top); } if (update_edge(riteE, local_bot)) { if (currE->fFirstY >= stop_y) { - break; + return; // we're done } riteE = currE; currE = currE->fNext; - } - - SkASSERT(leftE); - SkASSERT(riteE); - - // check our bottom clip - SkASSERT(local_top == local_bot + 1); - if (local_top >= stop_y) { - break; + ASSERT_RETURN(riteE->fFirstY == local_top); } } } @@ -503,9 +506,10 @@ void sk_fill_path(const SkPath& path, const SkIRect& clipRect, SkBlitter* blitte proc = PrePostInverseBlitterProc; } - if (path.isConvex() && (nullptr == proc)) { + // count >= 2 is required as the convex walker does not handle missing right edges + if (path.isConvex() && (nullptr == proc) && count >= 2) { SkASSERT(count >= 2); // convex walker does not handle missing right edges - walk_convex_edges(&headEdge, path.getFillType(), blitter, start_y, stop_y, nullptr); + walk_simple_edges(&headEdge, blitter, start_y, stop_y); } else { walk_edges(&headEdge, path.getFillType(), blitter, start_y, stop_y, proc, shiftedClip.right()); @@ -749,8 +753,7 @@ static void sk_fill_triangle(const SkPoint pts[], const SkIRect* clipRect, if (clipRect && start_y < clipRect->fTop) { start_y = clipRect->fTop; } - walk_convex_edges(&headEdge, SkPath::kEvenOdd_FillType, blitter, start_y, stop_y, nullptr); -// walk_edges(&headEdge, SkPath::kEvenOdd_FillType, blitter, start_y, stop_y, nullptr); + walk_simple_edges(&headEdge, blitter, start_y, stop_y); } void SkScan::FillTriangle(const SkPoint pts[], const SkRasterClip& clip, |