From c0a8dd4536e22e0bab6a5983a43f52c0e3f50de1 Mon Sep 17 00:00:00 2001 From: aplefull Date: Sun, 17 May 2026 01:20:49 +0200 Subject: [PATCH] LibRegex: Reject invalid /v class set expressions in negated classes Some string literals were incorrectly accepted inside negated classes because the negated class check computed the actual result string set instead of applying the structural rules. This commit replaces class_set_expression_strings with a structural class_set_expression_may_contain_strings check. Also removes some incorrect tests. --- Libraries/LibRegex/Rust/src/parser.rs | 94 +++++-------------- Tests/LibJS/Runtime/builtins/RegExp/RegExp.js | 7 +- .../regexp-unicode-sets-negated-class.js | 32 +++++++ .../Runtime/syntax/regex-literal-errors.js | 7 +- 4 files changed, 55 insertions(+), 85 deletions(-) create mode 100644 Tests/LibJS/Runtime/regexp-unicode-sets-negated-class.js diff --git a/Libraries/LibRegex/Rust/src/parser.rs b/Libraries/LibRegex/Rust/src/parser.rs index 0b01177b57..6b4f5a2c01 100644 --- a/Libraries/LibRegex/Rust/src/parser.rs +++ b/Libraries/LibRegex/Rust/src/parser.rs @@ -1182,7 +1182,7 @@ impl Parser { self.parse_class_set_expression()? }; self.in_negated_class = saved_negated; - if negated && !Self::class_set_expression_strings(&expr).is_empty() { + if negated && Self::class_set_expression_may_contain_strings(&expr) { return Err(Error::InvalidCharacterClass); } self.expect(']')?; @@ -1413,6 +1413,9 @@ impl Parser { self.in_negated_class = false; while self.peek_pair() == Some(('&', '&')) { self.pos += 2; // consume '&&' + if self.peek() == Some('&') { + return Err(Error::InvalidCharacterClass); + } operands.push(self.parse_class_set_operand()?); } self.in_negated_class = saved_negated; @@ -1454,93 +1457,38 @@ impl Parser { Ok(()) } - fn get_string_property_strings(name: &str) -> std::collections::BTreeSet> { - let buf = libunicode_rust::character_types::get_string_property_data(name); - if buf.is_empty() { - return std::collections::BTreeSet::new(); - } - - let count = buf[0] as usize; - let mut strings = std::collections::BTreeSet::new(); - let mut offset = 1; - for _ in 0..count { - if offset >= buf.len() { - break; - } - let len = buf[offset] as usize; - offset += 1; - if offset + len > buf.len() { - break; - } - if len > 1 { - strings.insert(buf[offset..offset + len].to_vec()); - } - offset += len; - } - strings - } - - fn class_set_expression_strings(expr: &ClassSetExpression) -> std::collections::BTreeSet> { + /// - Union: may contain strings if ANY operand may contain strings + /// - Intersection: may contain strings if ALL operands may contain strings + /// (i.e., no operand is "purely characters") + /// - Subtraction: may contain strings if the leftmost operand may contain strings + fn class_set_expression_may_contain_strings(expr: &ClassSetExpression) -> bool { match expr { - ClassSetExpression::Union(operands) => { - operands - .iter() - .fold(std::collections::BTreeSet::new(), |mut strings, operand| { - strings.extend(Self::class_set_operand_strings(operand)); - strings - }) - } + ClassSetExpression::Union(operands) => operands.iter().any(Self::class_set_operand_may_contain_strings), ClassSetExpression::Intersection(operands) => { - let Some((first, rest)) = operands.split_first() else { - return std::collections::BTreeSet::new(); - }; - let mut strings = Self::class_set_operand_strings(first); - for operand in rest { - let operand_strings = Self::class_set_operand_strings(operand); - strings.retain(|string| operand_strings.contains(string)); - } - strings - } - ClassSetExpression::Subtraction(operands) => { - let Some((first, rest)) = operands.split_first() else { - return std::collections::BTreeSet::new(); - }; - let mut strings = Self::class_set_operand_strings(first); - for operand in rest { - let operand_strings = Self::class_set_operand_strings(operand); - strings.retain(|string| !operand_strings.contains(string)); - } - strings + operands.iter().all(Self::class_set_operand_may_contain_strings) } + ClassSetExpression::Subtraction(operands) => operands + .first() + .is_some_and(Self::class_set_operand_may_contain_strings), } } - fn class_set_operand_strings(operand: &ClassSetOperand) -> std::collections::BTreeSet> { + fn class_set_operand_may_contain_strings(operand: &ClassSetOperand) -> bool { match operand { ClassSetOperand::NestedClass(class) => { if class.negated { - return std::collections::BTreeSet::new(); + return false; } match &class.body { - CharacterClassBody::Ranges(_) => std::collections::BTreeSet::new(), - CharacterClassBody::UnicodeSet(expr) => Self::class_set_expression_strings(expr), + CharacterClassBody::Ranges(_) => false, + CharacterClassBody::UnicodeSet(expr) => Self::class_set_expression_may_contain_strings(expr), } } ClassSetOperand::UnicodeProperty(property) => { - if !property.negated && property.value.is_none() && Self::is_string_property(&property.name) { - return Self::get_string_property_strings(&property.name); - } - std::collections::BTreeSet::new() - } - ClassSetOperand::StringLiteral(chars) => { - if chars.len() > 1 { - return [chars.iter().map(|ch| *ch as u32).collect()].into_iter().collect(); - } - std::collections::BTreeSet::new() - } - ClassSetOperand::Char(_) | ClassSetOperand::Range(_, _) | ClassSetOperand::BuiltinClass(_) => { - std::collections::BTreeSet::new() + !property.negated && property.value.is_none() && Self::is_string_property(&property.name) } + ClassSetOperand::StringLiteral(chars) => chars.len() != 1, + ClassSetOperand::Char(_) | ClassSetOperand::Range(_, _) | ClassSetOperand::BuiltinClass(_) => false, } } diff --git a/Tests/LibJS/Runtime/builtins/RegExp/RegExp.js b/Tests/LibJS/Runtime/builtins/RegExp/RegExp.js index d3d7d25f48..645ff5d807 100644 --- a/Tests/LibJS/Runtime/builtins/RegExp/RegExp.js +++ b/Tests/LibJS/Runtime/builtins/RegExp/RegExp.js @@ -36,12 +36,7 @@ describe("errors", () => { }); test("valid pattern (negated v-mode class set ops can eliminate strings)", () => { - for (const pattern of [ - "[^[[a-z]--[\\q{ab}]]]", - "[^[[\\q{ab}]&&[a-z]]]", - "[^[[\\q{ab}]--[\\q{ab}]]]", - "[^[[\\q{ab}]&&[\\q{cd}]]]", - ]) { + for (const pattern of ["[^[[a-z]--[\\q{ab}]]]", "[^[[\\q{ab}]&&[a-z]]]"]) { expect(() => { RegExp(pattern, "v"); }).not.toThrow(); diff --git a/Tests/LibJS/Runtime/regexp-unicode-sets-negated-class.js b/Tests/LibJS/Runtime/regexp-unicode-sets-negated-class.js new file mode 100644 index 0000000000..b197989858 --- /dev/null +++ b/Tests/LibJS/Runtime/regexp-unicode-sets-negated-class.js @@ -0,0 +1,32 @@ +test("triple & in unicode sets class is a syntax error", () => { + expect(() => eval("/[a&&&]/v")).toThrow(SyntaxError); + expect(() => eval("/[a&&b&&&c]/v")).toThrow(SyntaxError); +}); + +test("empty \\q{} in negated unicode sets class is a syntax error", () => { + expect(() => eval("/[^\\q{}]/v")).toThrow(SyntaxError); +}); + +test("multi-char string intersection where all operands contain strings is a syntax error in negated class", () => { + expect(() => eval("/[^\\q{foo}&&\\q{bar}]/v")).toThrow(SyntaxError); + expect(() => eval("/[^\\q{foo}&&\\q{foo}]/v")).toThrow(SyntaxError); +}); + +test("multi-char string in negated unicode sets class union is a syntax error", () => { + expect(() => eval("/[^\\q{ab}]/v")).toThrow(SyntaxError); + expect(() => eval("/[^\\q{foo}]/v")).toThrow(SyntaxError); +}); + +test("subtraction with string-valued left operand is a syntax error in negated class", () => { + expect(() => eval("/[^\\q{foo}--[a-z]]/v")).toThrow(SyntaxError); +}); + +test("valid unicode sets class set operations in negated class", () => { + expect(() => eval("/[^\\q{a}]/v")).not.toThrow(); + expect(() => eval("/[^\\q{foo}&&[a]]/v")).not.toThrow(); + expect(() => eval("/[^\\q{a}&&\\q{b}]/v")).not.toThrow(); + expect(() => eval("/[^\\q{a}&&[a-z]]/v")).not.toThrow(); + expect(() => eval("/[^[a-z]--\\q{foo}]/v")).not.toThrow(); + expect(() => eval("/[a&&b]/v")).not.toThrow(); + expect(() => eval("/[a&&b&&c]/v")).not.toThrow(); +}); diff --git a/Tests/LibJS/Runtime/syntax/regex-literal-errors.js b/Tests/LibJS/Runtime/syntax/regex-literal-errors.js index 545ea11b5b..5211153c85 100644 --- a/Tests/LibJS/Runtime/syntax/regex-literal-errors.js +++ b/Tests/LibJS/Runtime/syntax/regex-literal-errors.js @@ -89,12 +89,7 @@ test("negated v-mode classes containing nested strings are syntax errors", () => }); test("negated v-mode class set ops that eliminate strings are valid", () => { - for (const source of [ - "/[^[[a-z]--[\\q{ab}]]]/v", - "/[^[[\\q{ab}]&&[a-z]]]/v", - "/[^[[\\q{ab}]--[\\q{ab}]]]/v", - "/[^[[\\q{ab}]&&[\\q{cd}]]]/v", - ]) { + for (const source of ["/[^[[a-z]--[\\q{ab}]]]/v", "/[^[[\\q{ab}]&&[a-z]]]/v"]) { expect(source).toEval(); expect(() => eval(source)).not.toThrow(); expect(() => new Function(source)).not.toThrow();