Skip to content

Fix polynomial ReDoS in request-text parsing, tighten token file mode - #176

Merged
teamchong merged 1 commit into
mainfrom
security/code-scanning-followup
Aug 4, 2026
Merged

Fix polynomial ReDoS in request-text parsing, tighten token file mode#176
teamchong merged 1 commit into
mainfrom
security/code-scanning-followup

Conversation

@teamchong

@teamchong teamchong commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Nineteen code-scanning alerts. Four were real, fixed here. Fifteen were dismissed on the alert after measuring them.

Fixed

Three parsers walked untrusted request text with patterns that restart at every candidate position, so cost was quadratic in input length. Worst case is text that never satisfies the pattern, which is what a malformed or truncated body looks like.

Site Input Before After
<env> body 400KB, no closing tag 5.0s 0.7ms
static-tag scan '<A ' x80000 18.2s 0.1ms
minifyForRender one 400KB line of spaces 76.3s 0.1ms

The <env> and static-tag bodies used a lazy [\s\S]*? span, which rescans the tail once per candidate opening. The static-tag scan had a second, independent quadratic: matching openings with (?:\s[^>]*)?> rescans the tail once per candidate tag when no > follows. Fixing only the span left the alert open, which is what CodeQL caught on the first revision of this PR. Both now scan by index. minifyForRender walks back from the end of the line.

run_shard.py wrote the OAuth token file under the default umask and chmod'd it to 0600 afterwards, leaving it briefly readable by other users on the host. It is opened 0600 now. Caching the token on disk is deliberate, the proxy re-reads it per request so a task can outlive its token.

Dismissed

Twelve of the remaining ReDoS reports are anchored on a required literal prefix, which bounds the ambiguity. Measured flat under 1ms from 5KB to 640KB on matching, non-matching, and repeated-prefix inputs. Two of them (applicability.ts:23, gpt-model-profiles.ts:525) gate model selection and are deliberately identical to each other, so rewriting them to satisfy the scanner risks a routing regression for no measured gain. The proxy.ts and messages-chat-bridge.ts patterns normalize config URLs read at startup, not request data.

node.ts:1293 logs the cloudflare upstream URL, built only as .../accounts/${cfAccount}/ai/v1. That is the account id, not a credential. CLOUDFLARE_API_TOKEN is passed separately and never interpolated.

Checks

vitest run        52 files, 945 tests
tsc --noEmit      clean

Behaviour is unchanged, not just green. The rewrites were differentially fuzzed against the original regexes (500k cases for the tag scan, 400k for minifyForRender, no mismatches), and the hand-written character classifiers were compared against /[a-zA-Z0-9_-]/ and /\s/ across all 65536 BMP codepoints. That last check caught a real bug: I had included U+0085, which JS /\s/ does not match.

tests/redos-guard.test.ts fails on the unfixed code, the tag-scan guard at 15.3s against a 5s budget, so it is a regression net rather than a restatement of the implementation.

The four fixed alerts stay open until CodeQL rescans after merge.

Comment thread src/core/transform.ts Fixed
@teamchong
teamchong force-pushed the security/code-scanning-followup branch 2 times, most recently from 37cad9d to d1d11c1 Compare August 4, 2026 01:33
Three parsers walked untrusted request text with patterns that restart
at every candidate position, so cost was quadratic in input length. The
<env> and static-tag bodies used a lazy [\s\S]*? span, the static-tag
scan also matched openings with (?:\s[^>]*)?>, and minifyForRender
stripped line ends with /[ \t]+$/. Worst case is text that never
satisfies the pattern: 80k repeats of '<A ' took 18s, one 400KB line of
spaces took 76s.

All three scan by index instead. Behaviour is unchanged, checked by
differential fuzzing against the original regexes and by comparing the
new character classifiers against /[a-zA-Z0-9_-]/ and /\s/ across the
BMP.

run_shard.py wrote the OAuth token file under the default umask and
chmod'd it to 0600 afterwards, leaving it briefly readable by other
users on the host. Open it 0600 instead.
@teamchong
teamchong force-pushed the security/code-scanning-followup branch from d1d11c1 to 89fb91a Compare August 4, 2026 01:45
@teamchong
teamchong merged commit 9a7f1e6 into main Aug 4, 2026
6 checks passed
@teamchong
teamchong deleted the security/code-scanning-followup branch August 4, 2026 01:47
qbq-leo-martens pushed a commit to qbq-leo-martens/pxpipe that referenced this pull request Aug 5, 2026
…-history page breaks (teamchong#178), warp host:port routes (teamchong#175), macOS restart fix
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.

2 participants