check for irect with overflow width/height

Bug:798066
Change-Id: Iac324ac5a32fae241a528751c84279ce60ac4baf
Reviewed-on: https://skia-review.googlesource.com/90544
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>

5 files changed, 64 insertions, 11 deletions

diff --git a/include/core/SkRect.h b/include/core/SkRect.h
index e4ff612883..f4c700b34b 100644
--- a/include/core/SkRect.h
+++ b/include/core/SkRect.h
@@ -10,6 +10,7 @@
 
 #include "SkPoint.h"
 #include "SkSize.h"
+#include "../private/SkPedanticMath.h"
 #include "../private/SkTFitsIn.h"
 
 struct SkRect;
@@ -117,56 +118,56 @@ struct SK_API SkIRect {
 
         @return  fLeft
     */
-    int left() const { return fLeft; }
+    int32_t left() const { return fLeft; }
 
     /** Returns top edge of SkIRect, if sorted. Call isEmpty() to see if SkIRect may be invalid,
         and sort() to reverse fTop and fBottom if needed.
 
         @return  fTop
     */
-    int top() const { return fTop; }
+    int32_t top() const { return fTop; }
 
     /** Returns right edge of SkIRect, if sorted.
         Call sort() to reverse fLeft and fRight if needed.
 
         @return  fRight
     */
-    int right() const { return fRight; }
+    int32_t right() const { return fRight; }
 
     /** Returns bottom edge of SkIRect, if sorted. Call isEmpty() to see if SkIRect may be invalid,
         and sort() to reverse fTop and fBottom if needed.
 
         @return  fBottom
     */
-    int bottom() const { return fBottom; }
+    int32_t bottom() const { return fBottom; }
 
     /** Returns left edge of SkIRect, if sorted. Call isEmpty() to see if SkIRect may be invalid,
         and sort() to reverse fLeft and fRight if needed.
 
         @return  fLeft
     */
-    int x() const { return fLeft; }
+    int32_t x() const { return fLeft; }
 
     /** Returns top edge of SkIRect, if sorted. Call isEmpty() to see if SkIRect may be invalid,
         and sort() to reverse fTop and fBottom if needed.
 
         @return  fTop
     */
-    int y() const { return fTop; }
+    int32_t y() const { return fTop; }
 
     /** Returns span on the x-axis. This does not check if SkIRect is sorted, or if
         result fits in 32-bit signed integer; result may be negative.
 
         @return  fRight minus fLeft
     */
-    int width() const { return fRight - fLeft; }
+    int32_t width() const { return SkSub32(fRight, fLeft); }
 
     /** Returns span on the y-axis. This does not check if SkIRect is sorted, or if
         result fits in 32-bit signed integer; result may be negative.
 
         @return  fBottom minus fTop
     */
-    int height() const { return fBottom - fTop; }
+    int32_t height() const { return SkSub32(fBottom, fTop); }
 
     /** Returns spans on the x-axis and y-axis. This does not check if SkIRect is sorted,
         or if result fits in 32-bit signed integer; result may be negative.
@@ -182,7 +183,7 @@ struct SK_API SkIRect {
 
         @return  midpoint in x
     */
-    int centerX() const { return (fRight + fLeft) >> 1; }
+    int32_t centerX() const { return (fRight + fLeft) >> 1; }
 
     /** Returns average of top edge and bottom edge. Result does not change if SkRect
         is sorted. Result may be incorrect if SkRect is far from the origin.
@@ -191,7 +192,7 @@ struct SK_API SkIRect {
 
         @return  midpoint in y
     */
-    int centerY() const { return (fBottom + fTop) >> 1; }
+    int32_t centerY() const { return (fBottom + fTop) >> 1; }
 
     /** Returns true if fLeft is equal to or greater than fRight, or if fTop is equal
         to or greater than fBottom. Call sort() to reverse rectangles with negative
diff --git a/include/private/SkPedanticMath.h b/include/private/SkPedanticMath.h
new file mode 100644
index 0000000000..6d9c8a18be
--- /dev/null
+++ b/include/private/SkPedanticMath.h
@@ -0,0 +1,17 @@
+/*
+ * 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 SkPedanticMath_DEFINED
+#define SkPedanticMath_DEFINED
+
+// To avoid UBSAN complaints about 2's compliment overflows
+//
+static inline int32_t SkSub32(int32_t a, int32_t b) {
+    return (int32_t)((uint32_t)a - (uint32_t)b);
+}
+
+#endif
diff --git a/src/core/SkAAClip.cpp b/src/core/SkAAClip.cpp
index d4db8ee31e..c0707c5b0c 100644
--- a/src/core/SkAAClip.cpp
+++ b/src/core/SkAAClip.cpp
@@ -9,6 +9,7 @@
 #include "SkAtomics.h"
 #include "SkBlitter.h"
 #include "SkColorData.h"
+#include "SkRectPriv.h"
 #include "SkPath.h"
 #include "SkScan.h"
 #include "SkUtils.h"
@@ -703,7 +704,7 @@ bool SkAAClip::setEmpty() {
 }
 
 bool SkAAClip::setRect(const SkIRect& bounds) {
-    if (bounds.isEmpty()) {
+    if (!SkRectPriv::PositiveDimensions(bounds)) {
         return this->setEmpty();
     }
 
diff --git a/src/core/SkRectPriv.h b/src/core/SkRectPriv.h
new file mode 100644
index 0000000000..4dcc4c56ce
--- /dev/null
+++ b/src/core/SkRectPriv.h
@@ -0,0 +1,24 @@
+/*
+ * 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 SkRectPriv_DEFINED
+#define SkRectPriv_DEFINED
+
+#include "SkRect.h"
+
+class SkRectPriv {
+public:
+    // Returns true iff width and height are positive. Catches inverted, empty, and overflowing
+    // (way too big) rects. This is used by clients that want a non-empty rect that they can also
+    // actually use its computed width/height.
+    //
+    static bool PositiveDimensions(const SkIRect& r) {
+        return r.width() > 0 && r.height() > 0;
+    }
+};
+
+#endif
diff --git a/tests/AAClipTest.cpp b/tests/AAClipTest.cpp
index 5b156a40b5..18cc30c745 100644
--- a/tests/AAClipTest.cpp
+++ b/tests/AAClipTest.cpp
@@ -409,6 +409,15 @@ static void test_crbug_422693(skiatest::Reporter* reporter) {
     rc.op(path, SkMatrix::I(), rc.getBounds(), SkRegion::kIntersect_Op, true);
 }
 
+static void test_huge(skiatest::Reporter* reporter) {
+    SkAAClip clip;
+    int big = 0x70000000;
+    SkIRect r = { -big, -big, big, big };
+    SkASSERT(r.width() < 0 && r.height() < 0);
+
+    clip.setRect(r);
+}
+
 DEF_TEST(AAClip, reporter) {
     test_empty(reporter);
     test_path_bounds(reporter);
@@ -419,4 +428,5 @@ DEF_TEST(AAClip, reporter) {
     test_nearly_integral(reporter);
     test_really_a_rect(reporter);
     test_crbug_422693(reporter);
+    test_huge(reporter);
 }