Changeset 239833 in webkit

Timestamp:

Jan 10, 2019, 10:08:23 AM (8 years ago)

Author:

dino@apple.com

Message:

Safari Crashing in Version 12.0.1 (14606.2.104.1.1) WebCore::GraphicsLayerCA::updateBackdropFilters
​https://bugs.webkit.org/show_bug.cgi?id=193309
<rdar://problem/45279224>

Reviewed by Antoine Quint.

A speculative fix for a CheckedArithmetic crash triggered in updateBackdropFilters.

The crash log indicates we crash in a Checked<> class that is not recording
overflow i.e. it is crashing due to an overflow. The only place in this function
where that could happen is when we convert the FloatRect for the backdrop
region into a Checked<unsigned> for width and height. This suggests that either
the width or height are negative, or the float values are too large for integers,
or the product of the two overflows.

Avoid this by using RecordOverflow, but also changing the code a little to
bail if the rectangle is incorrect.

• platform/graphics/ca/GraphicsLayerCA.cpp:

(WebCore::GraphicsLayerCA::updateBackdropFilters):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/ca/GraphicsLayerCA.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-09  Dean Jackson  <dino@apple.com>
+
+        Safari Crashing in Version 12.0.1 (14606.2.104.1.1) WebCore::GraphicsLayerCA::updateBackdropFilters
+        https://bugs.webkit.org/show_bug.cgi?id=193309
+        <rdar://problem/45279224>
+
+        Reviewed by Antoine Quint.
+
+        A speculative fix for a CheckedArithmetic crash triggered in updateBackdropFilters.
+
+        The crash log indicates we crash in a Checked<> class that is not recording
+        overflow i.e. it is crashing due to an overflow. The only place in this function
+        where that could happen is when we convert the FloatRect for the backdrop
+        region into a Checked<unsigned> for width and height. This suggests that either
+        the width or height are negative, or the float values are too large for integers,
+        or the product of the two overflows.
+
+        Avoid this by using RecordOverflow, but also changing the code a little to
+        bail if the rectangle is incorrect.
+
+        * platform/graphics/ca/GraphicsLayerCA.cpp:
+        (WebCore::GraphicsLayerCA::updateBackdropFilters):
+
 2019-01-10  Oriol Brufau  <obrufau@igalia.com>
 

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -2148 +2148 @@
 void GraphicsLayerCA::updateBackdropFilters(CommitState& commitState)
 {
+    using CheckedUnsigned = Checked<unsigned, RecordOverflow>;
+
     bool canHaveBackdropFilters = needsBackdrop();
 
     if (canHaveBackdropFilters) {
-        Checked<unsigned, RecordOverflow> backdropFilterArea = Checked<unsigned>(static_cast<int>(m_backdropFiltersRect.rect().width())) * Checked<unsigned>(static_cast<int>(m_backdropFiltersRect.rect().height()));
-        if (backdropFilterArea.hasOverflowed())
-            canHaveBackdropFilters = false;
-        else {
-            Checked<unsigned, RecordOverflow> newTotalBackdropFilterArea = Checked<unsigned, RecordOverflow>(commitState.totalBackdropFilterArea) + backdropFilterArea;
-            if (newTotalBackdropFilterArea.hasOverflowed() || newTotalBackdropFilterArea.unsafeGet() > cMaxTotalBackdropFilterArea)
-                canHaveBackdropFilters = false;
-            else
-                commitState.totalBackdropFilterArea = newTotalBackdropFilterArea.unsafeGet();
+        canHaveBackdropFilters = false;
+        IntRect backdropFilterRect = enclosingIntRect(m_backdropFiltersRect.rect());
+        if (backdropFilterRect.width() > 0 && backdropFilterRect.height() > 0) {
+            CheckedUnsigned backdropFilterArea = CheckedUnsigned(backdropFilterRect.width()) * CheckedUnsigned(backdropFilterRect.height());
+            if (!backdropFilterArea.hasOverflowed()) {
+                CheckedUnsigned newTotalBackdropFilterArea = CheckedUnsigned(commitState.totalBackdropFilterArea) + backdropFilterArea;
+                if (!newTotalBackdropFilterArea.hasOverflowed() && newTotalBackdropFilterArea.unsafeGet() <= cMaxTotalBackdropFilterArea) {
+                    commitState.totalBackdropFilterArea = newTotalBackdropFilterArea.unsafeGet();
+                    canHaveBackdropFilters = true;
+                }
+            }
         }
     }