From da639a84525023bcd1bb861e828ce323f5db15a2 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 20 Sep 2026 16:03:08 -0700 Subject: [PATCH] fix ReDoS hang when expanding dimension shorthands RE_FUNCTIONS backtracked exponentially on unclosed CSS functions (e.g. `margin: x() calc(aaaa...)`), pinning a CPU on a few dozen bytes of input. - recursion-first alternation + possessive quantifiers: failed matches cost O(n) per attempt instead of O(2^n) - 10ms regexp timeout as backstop, falls back to plain whitespace splitting so hostile values mis-render instead of hanging --- CHANGELOG.md | 1 + lib/css_parser/regexps.rb | 35 ++++++++++++++--------- lib/css_parser/rule_set.rb | 14 ++++++--- test/test_rule_set_expanding_shorthand.rb | 26 +++++++++++++++++ 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4d6777..3f709e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Ruby CSS Parser CHANGELOG ### Unreleased +* Fix ReDoS hang when expanding dimension shorthands containing unclosed CSS functions (e.g. `margin: x() calc(aaaa...)`): RE_FUNCTIONS now uses possessive quantifiers plus a 10ms regexp timeout with fallback to plain whitespace splitting ### Version 3.1.0 * `Parser#load_uri!` accepts an `integrity:` option (Subresource Integrity, https://www.w3.org/TR/SRI/) to verify a fetched remote stylesheet before it is parsed diff --git a/lib/css_parser/regexps.rb b/lib/css_parser/regexps.rb index 911570b..c150785 100644 --- a/lib/css_parser/regexps.rb +++ b/lib/css_parser/regexps.rb @@ -61,20 +61,27 @@ def self.regex_possible_values(*values) RE_BORDER_UNITS = Regexp.union(BOX_MODEL_UNITS_RX, /(thin|medium|thick)/i) # Functions like calc, var, clamp, etc. - RE_FUNCTIONS = / - ( - [a-z0-9-]+ # function name - ) - (?> - \( # opening parenthesis - (?: - ([^()]+) - | # recursion via subexpression - \g<0> - )* - \) # closing parenthesis - ) - /imx.freeze + # Possessive quantifiers + recursion-first alternation avoid exponential + # backtracking on unclosed functions, the timeout is a backstop. + RE_FUNCTIONS = Regexp.new( + / + ( + [a-z0-9-]+ # function name + ) + (?> + \( # opening parenthesis + (?: + \g<0> # nested function + | + [^()a-z0-9]++ + | + [a-z0-9-]++ (?!\() + )* + \) # closing parenthesis + ) + /imx, + timeout: 0.01 + ).freeze # Patterns for specificity calculations NON_ID_ATTRIBUTES_AND_PSEUDO_CLASSES_RX_NC = / diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index 467f2b5..453c34f 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -696,10 +696,16 @@ def parse_selectors!(selectors) # :nodoc: end def split_value_preserving_function_whitespace(value) - split_value = value.gsub(RE_FUNCTIONS) do |c| - c.gsub!(/\s+/, WHITESPACE_REPLACEMENT) - c - end + # hostile values can still hit the regexp timeout, then split without protecting functions + split_value = + begin + value.gsub(RE_FUNCTIONS) do |c| + c.gsub!(/\s+/, WHITESPACE_REPLACEMENT) + c + end + rescue Regexp::TimeoutError + value + end matches = split_value.strip.split(/\s+/) diff --git a/test/test_rule_set_expanding_shorthand.rb b/test/test_rule_set_expanding_shorthand.rb index e0b7476..089a951 100644 --- a/test/test_rule_set_expanding_shorthand.rb +++ b/test/test_rule_set_expanding_shorthand.rb @@ -360,6 +360,32 @@ def test_functions_with_commas assert_equal expected_declarations, declarations end + def test_nested_functions + shorthand = 'margin: calc(min(1px, 2px) + 3px);' + declarations = expand_declarations(shorthand) + expected_declarations = { + 'margin-top' => 'calc(min(1px, 2px) + 3px)', + 'margin-bottom' => 'calc(min(1px, 2px) + 3px)', + 'margin-left' => 'calc(min(1px, 2px) + 3px)', + 'margin-right' => 'calc(min(1px, 2px) + 3px)' + } + assert_equal expected_declarations, declarations + end + + def test_unclosed_function_does_not_hang + # the balanced x() gets the value past the unmatched-parenthesis check in parse_declarations! + padding = 'a' * 10_000 + shorthand = "margin: x() calc(#{padding};" + declarations = expand_declarations(shorthand) + expected_declarations = { + 'margin-top' => 'x()', + 'margin-bottom' => 'x()', + 'margin-left' => "calc(#{padding}", + 'margin-right' => "calc(#{padding}" + } + assert_equal expected_declarations, declarations + end + protected def expand_declarations(declarations)