Skip to content

Fix regex backtracking into a nested counted loop - #2277

Open
JoaoPauloCMarra wants to merge 1 commit into
meta:static_hfrom
JoaoPauloCMarra:fix-regex-nested-loop-backtrack
Open

JoaoPauloCMarra wants to merge 1 commit into
meta:static_hfrom
JoaoPauloCMarra:fix-regex-nested-loop-backtrack

Conversation

@JoaoPauloCMarra

Copy link
Copy Markdown

Summary

Fixes #2232. Fixes #2235.

When the regex executor enters a counted loop (BeginLoop), it resets the loop's iteration count to 0 without saving the old value on the backtrack stack. If that loop is nested in another loop, the next outer iteration enters it again and overwrites the count from the previous entry. When a later failure backtracks into that previous entry (for example a greedy a+ giving back a character), the loop resumes with a count of 0, so a {1} loop runs its body again instead of exiting. Depending on the pattern this gives a wrong result or runs until the backtracking limit.

/(?:(?:a+){1}){2}/.test("aa")       // false, expected true
/(?:(a+){1}){2}/.exec("aaa")        // null, expected ["aaa", "a"]
/(?:(?:a?){1}){1,3}c/.exec("a")     // RangeError, expected null
/^(?:(?:(?:\W+){1,1})*\s)$/.test(String.fromCharCode(0x2028) + "\n")
                                    // false, expected true

The fix pushes a SetLoopData entry before resetting the count. The push is skipped when the count is already 0, because the reset then changes nothing. So the only cost is one backtrack entry when a loop with a non-zero count is entered again, which only happens for loops nested in other loops. Simple loops and width-1 loops do not use LoopData and are not affected.

Test Plan

Added test/hermes/regexp-nested-loop-backtrack.js. It covers both issues (the ASCII and UTF-16 matchers for #2235), capture groups, a failed first start position, a non-greedy outer loop, a lookbehind and the RangeError case. Without the fix 10 of its 11 checks fail. With the fix all pass, and every expected value matches V8.

ASan Debug build (-O1) on macOS arm64:

cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DHERMES_ENABLE_ADDRESS_SANITIZER=ON -DCMAKE_CXX_FLAGS=-O1 -DCMAKE_C_FLAGS=-O1
cmake --build build --target check-hermes
python3 utils/test_runner.py -b build/bin test262/test/built-ins/RegExp test262/test/language/literals/regexp test262/test/built-ins/String/prototype/{match,matchAll,replace,replaceAll,search,split}
  • check-hermes: 4364 passed, 0 unexpected failures
  • test262 at the CI-pinned revision (c9aa6126), the directories above: 2163 passed, 0 failed, both with and without the fix. The skipped tests that pass in those directories are the same with and without the fix.
  • Differential fuzzing against V8 with 40,000 unique random (pattern, input) pairs, half of them biased toward nested and non-greedy loops: this change fixes 209 mismatches and introduces none.

BeginLoop reset the loop's iteration count to 0 without saving the old
value. When an outer loop entered the inner loop again, backtracking
into the inner loop's previous entry resumed with a count of 0, so a
{1} loop ran its body again instead of exiting. For example,
/(?:(?:a+){1}){2}/ did not match "aa", and /(?:(?:a?){1}){1,3}c/
threw RangeError on "a".

Save the loop state on the backtrack stack before resetting it when the
count is not already 0.

Fixes meta#2232
Fixes meta#2235
@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 29, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regex fails to match line separator followed by newline regex nested counted repetition fails to match

1 participant