fix(security): bound the _URL_QUERY_STRING_RE path segment — quadratic backtracking on many-`://` scrub_error inputs

Author: LearningCircuitCreated Sep 17, 2026Updated Sep 18, 2026
Labelssecuritypriority: highpythonseverity:should-fixfrom-review-extractionfollow-up-work-complete

Summary

The URL query-string redaction pattern added in #6422 bounds the scheme continuation ({0,31}) but leaves the path segment [^\s?#]+ unbounded. On a whitespace-free token containing many :// occurrences and no ?, the greedy path match backtracks one character at a time from every scheme-match offset — quadratic total work on the attacker-influenceable scrub_error() path, the same CPU-bound DoS class the PR set out to close. The merged timing test uses an input with no ://, so it only guards the already-fixed scheme bound.

Field Value
Severity should-fix
Origin Review of #6422 (post-merge extraction)
Review section Should-fix
Classification PR-introduced
Verified at main@caf15bdf2617
Dedup distinct · 12 hits reviewed

Trigger

Measured at main@caf15bdf2617 against the merged pattern ([A-Za-z][A-Za-z0-9+.\-]{0,31}://[^\s?#]+)\?[^\s)\]\}\"']+, calling .sub() on a message of "a://" * k (no ? anywhere):

input time
"a://" * 500 (2 KB) 3.97 ms
"a://" * 1,000 (4 KB) 13.5 ms
"a://" * 2,000 (8 KB) 60.7 ms
"a://" * 4,000 (16 KB) 247 ms
"a://" * 8,000 (32 KB) 816 ms

Clean ~4× per doubling — quadratic. Extrapolates to ~14 minutes of CPU for a ~1 MB exception string (str(error) containing "a://" * 250_000, or a minified/attacker-controlled blob with many embedded http:// substrings and no ?). Control: the scheme-run input the merged test uses ("a" * 200_000, no ://) scrubs in 12.6 ms — only the many-:// shape regresses.

Impact

A single scrub_error() call on such a message hangs the worker CPU-bound for minutes — a self-DoS on the exact path that exists to scrub attacker-influenceable exception text (search-engine errors echoing response bodies or long URL lists). Same loss class the PR's {0,31} scheme bound fixed for the adjacent mechanism; entered here through the second, still-unbounded segment.

Pinned evidence

  • caf15bdf2617:src/local_deep_research/security/log_sanitizer.py:L318-L320 — the pattern: scheme continuation bounded {0,31}, path segment [^\s?#]+ unbounded
  • caf15bdf2617:src/local_deep_research/security/log_sanitizer.py:L300-L307 — the comment documents the scheme-continuation quadratic and its fix; the path-segment twin is unaddressed
  • caf15bdf2617:tests/security/test_log_sanitizer.py:L651-L678 — timing test whose input run = "a" * 200_000 contains no ://; cannot catch this path

Fix direction

Bound the path segment (e.g. [^\s?#]{1,512}) so worst-case work is linear in input × a small constant, or pre-split the message on whitespace / locate ? positions first and validate a bounded prefix. Then extend the timing test (or add a sibling) whose input is a long repeated "a://" / "http://" run with no ?, under the same @pytest.mark.timeout(5) backstop.

Targeted tests

  • Timing: "a://" * 50_000 (and a 2× size for a scaling ratio) completes under a generous budget; a regression to unbounded is killed by timeout(5) rather than hanging CI
  • Behavior unchanged: x://host/path?term=secret still redacts to x://host/path?<redacted> (existing test_one_character_scheme_is_covered guards the scheme side)

Provenance

Extracted from review of PR #6422 (post-merge extraction). Related: #6482, #6266, #6403.

Source: LearningCircuit/local-deep-research