Skip to content

fix(chat): remove quadratic backtracking from two regexes - #58

Merged
rpvilo merged 4 commits into
mainfrom
feature/fix-redos
Aug 3, 2026
Merged

fix(chat): remove quadratic backtracking from two regexes#58
rpvilo merged 4 commits into
mainfrom
feature/fix-redos

Conversation

@rpvilo

@rpvilo rpvilo commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Closes the two high-severity CodeQL js/polynomial-redos alerts. Both are in shipped package code, and one is reachable from untrusted input.

parseChipSegments — the serious one

CHIP_REF_PATTERN's label group was [^\]]+. Given text with a long run of [ and no closing bracket, the engine consumed to end-of-string, failed, backtracked over every position, advanced one character and repeated — quadratic in the text length.

It matters because message text is untrusted: it comes from the model. Message.Text calls parseChipSegments on it directly, so an adversarial response could freeze a consumer's tab.

Measured, old pattern:

input time
2 000 2.2 ms
8 000 26.4 ms
32 000 354.9 ms

4× the input for 12× the time, then 16× for 161× — textbook quadratic. Extrapolated to 200 000 characters that's roughly fourteen seconds. The new pattern is flat at 0.0 ms across all three sizes.

The fix excludes [ from the label class. No behaviour change: I verified the round-trip both ways, and labels containing brackets already failed to match under the old pattern, because escapeMarkdownLink emits \[…\] and the old class excluded ] regardless of the backslash:

"plain"    → old: ["plain"]           new: ["plain"]
"a[b]c"    → old: []                  new: []            (already broken)
"paren(x)" → old: ["paren\\(x\\)"]    new: ["paren\\(x\\)"]

detectActivePrefix — same shape, self-inflicted

leftRun used /\S*$/, which retries from every position when the text ends in whitespace. Input is the user's own composer text, so it isn't remotely triggerable — but it runs on every keystroke through the trigger tracker, so pasting a large single-token blob froze the editor.

Measured on a 200 000-character run followed by a space: 16 414 ms old, 0.0 ms new, with byte-identical results. Replaced with a backwards index scan — linear, allocation-free, and it also removed a slice allocation from the word-boundary check that followed.

The /^\S*/ on the other side is start-anchored and cannot backtrack, so it stays.

Regression tests

Both are covered, and I checked the tests are actually load-bearing rather than passing either way — measured out-of-band, the old implementations exceed the 100 ms budgets by 164× and 138× respectively. Also added a test that a bracket run followed by a real chip still finds the chip, so the tightened class can't silently over-restrict.

Also here

ci.yml gains an explicit permissions: contents: read, closing the medium actions/missing-workflow-permissions alert. The repo default is already read-only; stating it means a change to that default can't silently widen CI.

Not in scope

  • js/incomplete-sanitization at composer-prefix.test.ts:11 is a false positive — the helper replaces a single caret marker on purpose. Worth dismissing in the UI rather than "fixing".
  • A pre-existing round-trip bug surfaced while verifying the above: escaped characters aren't unescaped on parse, so a label containing ( comes back as paren\(x\). Unrelated to ReDoS, no security impact, wants its own PR.

Verified

182 tests (3 new), tsc --noEmit clean, biome clean, package build + publint clean, app builds. Changeset resolves to patch → 0.1.1.

@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
intentface-chat Ready Ready Preview Aug 3, 2026 9:14pm

Request Review

@rpvilo
rpvilo merged commit 9f74772 into main Aug 3, 2026
6 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant