[@Property] Nullptr crash with calc()

https://bugs.webkit.org/show_bug.cgi?id=256032 rdar://105491386 Reviewed by Alan Baradlay. * LayoutTests/fast/css/custom-properties/at-property-calc-crash.html: Added. * LayoutTests/fast/css/custom-properties/at-property-calc-crash-expected.txt: Added. * Source/WebCore/css/CSSCustomPropertyValue.cpp: (WebCore::CSSCustomPropertyValue::customCSSText const): Ensure that we don't crash even if the calc expression building returns null. * Source/WebCore/css/calc/CSSCalcValue.cpp: (WebCore::createCSS): Limit zero-length elimination when constructing CSSCalcExpressionNodes from CalcExpressionNodes to sum and substract expressions. With other expression types eliminating zeroes can lead to miscomputing the expression unit category and the building code returning null. Canonical link: https://commits.webkit.org/263453@main
WebKit · Apr 27, 2023 · 8b62fda · 8b62fda
1 parent af2e0ab
commit 8b62fda
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 4 deletions.
diff --git a/LayoutTests/fast/css/custom-properties/at-property-calc-crash-expected.txt b/LayoutTests/fast/css/custom-properties/at-property-calc-crash-expected.txt
@@ -0,0 +1 @@
+This test passes it it doesn't crash.
diff --git a/LayoutTests/fast/css/custom-properties/at-property-calc-crash.html b/LayoutTests/fast/css/custom-properties/at-property-calc-crash.html
@@ -0,0 +1,16 @@
+<style>
+  body {
+    --v: calc(0% + 1px + 0 * max(0px, 0cqi));
+    width: var(--v);
+  }
+  @property --v {
+    syntax: '<length-percentage>';
+    inherits: false;
+    initial-value: 0;
+  }
+</style>
+<script>
+if (window.testRunner)
+  testRunner.dumpAsText();
+</script>
+This test passes it it doesn't crash.
diff --git a/Source/WebCore/css/CSSCustomPropertyValue.cpp b/Source/WebCore/css/CSSCustomPropertyValue.cpp
@@ -71,6 +71,14 @@ String CSSCustomPropertyValue::customCSSText() const
 {
     auto serializeSyntaxValue = [](const SyntaxValue& syntaxValue) -> String {
         return WTF::switchOn(syntaxValue, [&](const Length& value) {
+            if (value.type() == LengthType::Calculated) {
+                auto calcValue = CSSCalcValue::create(value.calculationValue(), RenderStyle::defaultStyle());
+                if (!calcValue) {
+                    ASSERT_NOT_REACHED();
+                    return emptyString();
+                }
+                return calcValue->cssText();
+            }
             return CSSPrimitiveValue::create(value, RenderStyle::defaultStyle())->cssText();
         }, [&](const NumericSyntaxValue& value) {
             return CSSPrimitiveValue::create(value.value, value.unitType)->cssText();

diff --git a/Source/WebCore/css/calc/CSSCalcValue.cpp b/Source/WebCore/css/calc/CSSCalcValue.cpp
@@ -72,6 +72,23 @@ static Vector<Ref<CSSCalcExpressionNode>> createCSS(const Vector<std::unique_ptr
     });
 }
 
+static RefPtr<CSSCalcExpressionNode> createCSSIgnoringZeroLength(const CalcExpressionNode& node, const RenderStyle& style)
+{
+    if (node.type() == CalcExpressionNodeType::Length) {
+        auto& length = downcast<CalcExpressionLength>(node).length();
+        if (!length.isPercent() && length.isZero())
+            return nullptr;
+    }
+    return createCSS(node, style);
+}
+
+static Vector<Ref<CSSCalcExpressionNode>> createCSSIgnoringZeroLengths(const Vector<std::unique_ptr<CalcExpressionNode>>& nodes, const RenderStyle& style)
+{
+    return WTF::compactMap(nodes, [&](auto& node) -> RefPtr<CSSCalcExpressionNode> {
+        return createCSSIgnoringZeroLength(*node, style);
+    });
+}
+
 static RefPtr<CSSCalcExpressionNode> createCSS(const CalcExpressionNode& node, const RenderStyle& style)
 {
     switch (node.type()) {
@@ -81,8 +98,6 @@ static RefPtr<CSSCalcExpressionNode> createCSS(const CalcExpressionNode& node, c
     }
     case CalcExpressionNodeType::Length: {
         auto& length = downcast<CalcExpressionLength>(node).length();
-        if (!length.isPercent() && length.isZero())
-            return nullptr;
         return createCSS(length, style);
     }
 
@@ -105,7 +120,7 @@ static RefPtr<CSSCalcExpressionNode> createCSS(const CalcExpressionNode& node, c
 
         switch (op) {
         case CalcOperator::Add: {
-            auto children = createCSS(operationChildren, style);
+            auto children = createCSSIgnoringZeroLengths(operationChildren, style);
             if (children.isEmpty())
                 return nullptr;
             if (children.size() == 1)
@@ -119,7 +134,7 @@ static RefPtr<CSSCalcExpressionNode> createCSS(const CalcExpressionNode& node, c
             values.reserveInitialCapacity(operationChildren.size());
 
             auto firstChild = createCSS(*operationChildren[0], style);
-            auto secondChild = createCSS(*operationChildren[1], style);
+            auto secondChild = createCSSIgnoringZeroLength(*operationChildren[1], style);
 
             if (!secondChild)
                 return firstChild;