From 1f2629f132a400b3b0002229ce54fd9b96452fa4 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@serenityos.org>
Date: Fri, 2 Jun 2023 15:15:45 +0100
Subject: [PATCH] LibWeb: Bounds-check parsed CSS types

This reintroduces bounds-checking for the CSS `<angle>`, `<frequency>`,
`<integer>`, `<length>`, `<number>`, `<percentage>`, `<resolution>`,
and `<time>` types.

I regressed this around 6b8f4841145a53553c0007a6ff4feefec98a426a when
changing how we parsed StyleValues.

This is an improvement from before though, since we now allow the bounds
of a dimension type to have units.

Added a test to make sure we don't regress this again. :^)
---
 .../Layout/expected/negative-max-size.txt     |  8 +++++
 .../Layout/input/negative-max-size.html       |  9 +++++
 .../Libraries/LibWeb/CSS/Parser/Parser.cpp    | 34 +++++++++++--------
 3 files changed, 37 insertions(+), 14 deletions(-)
 create mode 100644 Tests/LibWeb/Layout/expected/negative-max-size.txt
 create mode 100644 Tests/LibWeb/Layout/input/negative-max-size.html
diff --git a/Tests/LibWeb/Layout/expected/negative-max-size.txt b/Tests/LibWeb/Layout/expected/negative-max-size.txt
new file mode 100644
index 0000000000..a38c7f13e8
--- /dev/null
+++ b/Tests/LibWeb/Layout/expected/negative-max-size.txt
@@ -0,0 +1,8 @@
+Viewport <#document> at (0,0) content-size 800x600 children: not-inline
+  BlockContainer <html> at (0,0) content-size 800x35.46875 [BFC] children: not-inline
+    BlockContainer <body> at (8,8) content-size 784x19.46875 children: not-inline
+      BlockContainer <div> at (9,9) content-size 782x17.46875 children: inline
+        line 0 width: 147.1875, height: 17.46875, bottom: 17.46875, baseline: 13.53125
+          frag 0 from TextNode start: 0, length: 20, rect: [9,9 147.1875x17.46875]
+            "Well, hello friends!"
+        TextNode <#text>
diff --git a/Tests/LibWeb/Layout/input/negative-max-size.html b/Tests/LibWeb/Layout/input/negative-max-size.html
new file mode 100644
index 0000000000..1a3c7348b1
--- /dev/null
+++ b/Tests/LibWeb/Layout/input/negative-max-size.html
@@ -0,0 +1,9 @@
+<!doctype html><style>
+    * {
+        font: 16px SerenitySans;
+    }
+    div {
+        border: 1px solid black;
+        max-width: -90px;
+    }
+</style><div>Well, hello friends!</div>
\ No newline at end of file
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
index 3f7f6ec6cd..10fe50eae0 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
@@ -7589,19 +7589,20 @@ ErrorOr<Parser::PropertyAndValue> Parser::parse_css_value_for_properties(Readonl
 
     if (peek_token.is(Token::Type::Number) && property_accepts_numeric) {
         if (property_accepting_integer.has_value()) {
-            if (auto integer = TRY(parse_integer_value(tokens)))
+            if (auto integer = TRY(parse_integer_value(tokens)); integer && property_accepts_integer(*property_accepting_integer, integer->as_integer().integer()))
                 return PropertyAndValue { *property_accepting_integer, integer };
         }
         if (property_accepting_number.has_value()) {
-            if (auto number = TRY(parse_number_value(tokens)))
+            if (auto number = TRY(parse_number_value(tokens)); number && property_accepts_number(*property_accepting_number, number->as_number().number()))
                 return PropertyAndValue { *property_accepting_number, number };
         }
     }
 
     if (peek_token.is(Token::Type::Percentage)) {
-        if (auto property = any_property_accepts_type(property_ids, ValueType::Percentage); property.has_value()) {
+        auto percentage = Percentage(peek_token.token().percentage());
+        if (auto property = any_property_accepts_type(property_ids, ValueType::Percentage); property.has_value() && property_accepts_percentage(*property, percentage)) {
             (void)tokens.next_token();
-            return PropertyAndValue { *property, TRY(PercentageStyleValue::create(Percentage(peek_token.token().percentage()))) };
+            return PropertyAndValue { *property, TRY(PercentageStyleValue::create(percentage)) };
         }
     }
 
@@ -7635,24 +7636,29 @@ ErrorOr<Parser::PropertyAndValue> Parser::parse_css_value_for_properties(Readonl
             (void)tokens.next_token();
             auto dimension = maybe_dimension.release_value();
             if (dimension.is_angle()) {
-                if (auto property = any_property_accepts_type(property_ids, ValueType::Angle); property.has_value())
-                    return PropertyAndValue { *property, TRY(AngleStyleValue::create(dimension.angle())) };
+                auto angle = dimension.angle();
+                if (auto property = any_property_accepts_type(property_ids, ValueType::Angle); property.has_value() && property_accepts_angle(*property, angle))
+                    return PropertyAndValue { *property, TRY(AngleStyleValue::create(angle)) };
             }
             if (dimension.is_frequency()) {
-                if (auto property = any_property_accepts_type(property_ids, ValueType::Frequency); property.has_value())
-                    return PropertyAndValue { *property, TRY(FrequencyStyleValue::create(dimension.frequency())) };
+                auto frequency = dimension.frequency();
+                if (auto property = any_property_accepts_type(property_ids, ValueType::Frequency); property.has_value() && property_accepts_frequency(*property, frequency))
+                    return PropertyAndValue { *property, TRY(FrequencyStyleValue::create(frequency)) };
             }
             if (dimension.is_length()) {
-                if (auto property = any_property_accepts_type(property_ids, ValueType::Length); property.has_value())
-                    return PropertyAndValue { *property, TRY(LengthStyleValue::create(dimension.length())) };
+                auto length = dimension.length();
+                if (auto property = any_property_accepts_type(property_ids, ValueType::Length); property.has_value() && property_accepts_length(*property, length))
+                    return PropertyAndValue { *property, TRY(LengthStyleValue::create(length)) };
             }
             if (dimension.is_resolution()) {
-                if (auto property = any_property_accepts_type(property_ids, ValueType::Resolution); property.has_value())
-                    return PropertyAndValue { *property, TRY(ResolutionStyleValue::create(dimension.resolution())) };
+                auto resolution = dimension.resolution();
+                if (auto property = any_property_accepts_type(property_ids, ValueType::Resolution); property.has_value() && property_accepts_resolution(*property, resolution))
+                    return PropertyAndValue { *property, TRY(ResolutionStyleValue::create(resolution)) };
             }
             if (dimension.is_time()) {
-                if (auto property = any_property_accepts_type(property_ids, ValueType::Time); property.has_value())
-                    return PropertyAndValue { *property, TRY(TimeStyleValue::create(dimension.time())) };
+                auto time = dimension.time();
+                if (auto property = any_property_accepts_type(property_ids, ValueType::Time); property.has_value() && property_accepts_time(*property, time))
+                    return PropertyAndValue { *property, TRY(TimeStyleValue::create(time)) };
             }
         }
     }
