Skip to content

fix(webdav): remove polynomial-redos in Basic auth header regex - #439

Merged
davidwkeith merged 1 commit into
mainfrom
claude/polynomial-regex-uncontrolled-data-idnkxt
Jul 24, 2026
Merged

fix(webdav): remove polynomial-redos in Basic auth header regex#439
davidwkeith merged 1 commit into
mainfrom
claude/polynomial-regex-uncontrolled-data-idnkxt

Conversation

@davidwkeith

@davidwkeith davidwkeith commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes GitHub code scanning alert #11 (js/polynomial-redos, CodeQL):
parseBasicAuthorization's regex in packages/webdav/src/credentials.ts had
\s+ immediately before (.+)$. Since \s is a subset of what . matches,
the two quantifiers were ambiguous about how to split the client-supplied
Authorization header between them — the same shape already fixed in
@dwk/webmention's parseLinkHeader (#436).

Fix: replaced the combined regex with a plain search()/slice() split on
the first whitespace run, which is unambiguous by construction (a linear
scan, no backtracking). Behavior is unchanged, confirmed by the existing
parseBasicAuthorization test suite (4/4 passing, 128/128 across the whole
@dwk/webdav package).

I also investigated the other four currently-open alerts of this same title
(#8/#9 in packages/mastodon-api/src/sanitize.ts:139,144, #10 in
packages/webauthn/src/encoding.ts:42, #12 in
packages/webdav/src/locks.ts:97). All four regexes involved use only a
single quantifier per character class with no adjacent-quantifier ambiguity
(e.g. /=+$/, /^\/+/, or the sticky, mutually-exclusive-character-class
tokenizer in sanitize.ts that already has extensive complexity-proof
comments from a prior ReDoS hardening pass). I stress-tested each against
adversarial multi-hundred-KB inputs locally and saw flat, linear timing —
they don't reproduce backtracking blowup, so I've left them as-is rather than
changing working code without a reproducible finding. Flagging them here in
case they should be dismissed as false positives on the CodeQL side.

Packages affected

  • @dwk/webdav

Checklist

  • Read the relevant spec(s) under spec/packages/ and updated them if behaviour changed — not applicable, no behavior change, pure regex hardening
  • Added/updated colocated tests (src/*.test.ts) — not applicable, existing credentials.test.ts coverage already exercises parseBasicAuthorization and continues to pass
  • Ran the local CI gate: pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm testlint/format:check ran locally, repo-wide; typecheck/build/test ran locally scoped to @dwk/webdav only (128/128 passing). The full repo-wide typecheck/build/test sequence CONTRIBUTING.md's gate calls for was covered by CI's build-test job instead, which is green on this PR.
  • Added a changeset (pnpm changeset) if this touches a publishable package
  • Updated catalog.json / conformance/status.json if this adds a new mountable worker or changes conformance status — not applicable, no new worker or conformance status change

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).

@davidwkeith davidwkeith left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small, well-scoped ReDoS fix that follows the same pattern already established in #436. Verified the replacement search/slice split against the old regex across the relevant edge cases (missing separator, wrong auth scheme, multiple whitespace, trailing-whitespace-only header) — behavior is preserved. Changeset included, existing test coverage (credentials.test.ts) continues to pass, CI is green, and the PR template's Summary/Packages affected/Checklist sections match CONTRIBUTING.md's structure. Left one non-blocking nit on the CI-gate checklist wording. No changes requested.


Generated by Claude Code

Comment thread packages/webdav/src/credentials.ts
@davidwkeith
davidwkeith merged commit a20ddcf into main Jul 24, 2026
8 checks passed
@davidwkeith
davidwkeith deleted the claude/polynomial-regex-uncontrolled-data-idnkxt branch July 24, 2026 17:05
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