fix(webmention): remove polynomial-redos in Link header regex - #436
Conversation
CodeQL (js/polynomial-redos) flagged the `\s*` immediately before `(.*)$` in parseLinkHeader's regex: since `\s` is a subset of what `.` matches, a crafted Link header value (attacker-controlled, fetched during webmention discovery/verification) with runs of whitespace could force excessive backtracking. Drop the redundant `\s*` — leading whitespace on each parameter is already trimmed by extractRel's per-parameter `rel` regex.
davidwkeith
left a comment
davidwkeith
left a comment
There was a problem hiding this comment.
Reviewed the diff and the surrounding html.ts parsing logic.
Correctness: the fix is sound. Removing the redundant \s* before (.*)$ eliminates the ambiguous-quantifier ReDoS (\s is a subset of ., so the two greedy quantifiers could split the match in exponentially many ways on crafted whitespace runs). Behavior is unchanged: splitParams already tokenizes on ;, and extractRel's own ^\s*rel\s*=...$ regex strips any leading whitespace on the captured parameter string, so the second capture group losing its explicit whitespace-trim doesn't change what extractRel/splitTokens ultimately see. Traced the whole path (splitLinks → parseLinkHeader → extractRel → splitTokens) and confirmed no other caller depends on match[2] being pre-trimmed.
CONTRIBUTING.md conformance:
- PR title
fix(webmention): remove polynomial-redos in Link header regex— correct Conventional Commits form (lowercase type, parenthesized scope, uncapitalized subject, no trailing period). - Changeset added (
@dwk/webmentionpatch) — appropriate bump for a behavior-preserving security hardening fix. - Template headings (
Summary/Packages affected/Checklist) kept verbatim; unchecked items carry one-line reasons rather than being deleted, per the contributing guide. - Scope is minimal and focused — single-line regex change plus changeset, no unrelated cleanup.
No changes requested. This looks ready to merge as-is.
Generated by Claude Code
|
The Job logs (run 30105424542) show it failing at the model-selection step, before analyzing any code: It's requesting Not blocking on this PR's actual content — the Generated by Claude Code |
parseBasicAuthorization's /^basic\s+(.+)$/i had \s+ immediately before (.+)$, ambiguous over runs of whitespace in the same way the already-fixed webmention Link header regex (#436) was. Replaced with an unambiguous search()/slice() split. Closes GitHub code scanning alert #11 (js/polynomial-redos). Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Summary
Fixes GitHub code scanning alert #13 (
js/polynomial-redos, CodeQL):parseLinkHeader's regex inpackages/webmention/src/html.tshad\s*immediately before(.*)$. Since\sis a subset of what.matches, the two quantifiers were ambiguous about how to split the input between them, allowing a crafted HTTPLinkheader value — attacker-controlled, since it's fetched from whatever server a webmention source/target points at — to force excessive (polynomial) regex backtracking.Fix: drop the redundant
\s*. Leading whitespace on each;-delimited parameter is already stripped byextractRel's own per-parameter^\s*rel\s*=...$regex, so parsing behavior is unchanged — confirmed by the existingparseLinkHeadertest suite, which still passes.Packages affected
@dwk/webmentionChecklist
spec/packages/and updated them if behaviour changed — not applicable, no behavior change, pure regex hardeningsrc/*.test.ts) — not applicable, existinghtml.test.tscoverage already exercisesparseLinkHeaderand continues to passpnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm testpnpm changeset) if this touches a publishable packagecatalog.json/conformance/status.jsonif this adds a new mountable worker or changes conformance status — not applicable, no new worker or conformance status changeGenerated by Claude Code