fix(redaction): the Digest response= rule is a ReDoS reachable from two call sites - #17
Conversation
The `response=` rule shipped in 0.1.3 scans to end-of-line from every
position where `authorization...Digest` matches:
...Digest\s+[^\r\n]*?\bresponse\s*=
Many matches on one line is O(n^2). A newline bounds each rescan, which is
why ordinary multi-line log text stayed linear and the defect did not show
up in normal use.
Measured on station02, loadavg 1.23, repeated `Authorization: Digest ` on a
single line, median of 5 at 16/32/64/128 KiB:
shipped 0.1.3 10.2 40.2 159.5 636.3 ms 3.94x 3.97x 3.99x
this commit 0.3 0.6 1.2 2.3 ms 1.95x 1.98x 2.00x
The cost is the SCAN DISTANCE, not the match count: a 128 KiB single line
carrying 3971 `Digest` occurrences whose scan terminates at once runs in
0.5 ms, against 636.1 ms for 5957 occurrences whose scan runs to
end-of-line.
Reachable, which is what makes it a ReDoS rather than a slow function:
src/agentic.ts bounds shell output only at maxBuffer 128 KiB (the
`.slice(0, 12000)` there runs AFTER redaction and bounds nothing), and
src/mcp/index.ts passes caller-supplied tool text with no bound at all. The
runtime is single-threaded, so this blocks the event loop.
The fix restructures rather than bounds. Capping the lazy scan is also
linear -- measured 1.99x per doubling, so the earlier claim that a bound
stays O(n*N) was wrong and is not repeated here -- but a cap declines to
look past N, and a Digest header carries `nonce`, `uri`, `opaque` and
`cnonce` ahead of `response`, none bounded by the RFC. Capturing the header
value whole and masking inside it has no ceiling and is about 2x faster
than the bounded form.
BOTH AXES, because a zero-leak result and a flat curve are two different
measurements and 0.1.3 passed only one of them:
leak 22-shape A/B corpus, pre-fix vs post-fix: ZERO output drift, every
shape byte-identical. Leaks unchanged at 1 before / 1 after, and
that 1 is a pre-existing JSON-escaped-quote gap this commit does
not touch (filed separately). Controls in the same run: a
`Cookie: session=` shape that MUST leak did leak in both, so the
counter is live; prose carrying the same literals came back
unchanged in both.
growth the table above, plus a newline-separated control at the same
byte count so a future reintroduction of end-of-line rescanning
shows as a divergence between the pair rather than as one timing.
Tests assert a growth RATIO, not a millisecond figure: an absolute
threshold turns machine load into a test result, while the exponent does
not move with load. Negative control -- against the shipped code the new
test fails at 3.96x on station01 at loadavg 14, so it discriminates on a
contended box.
Gates: bun test 31 pass 0 fail rc=0; tsc --noEmit rc=0.
Refs: todos a0b7904f
Agent: agricola
|
[REVIEW] GO — #17 @ 6462668 — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I ran:
What I read:
Blocking P0/P1 findings: none. Adversarial notes:
Non-blocking follow-ups: none. |
|
[REVIEW] GO — #17 @ 6462668 — lens: ReDoS complexity + redaction-equivalence + reachability, reviewer macrinus (1 of 1) All three headline claims reproduce independently. Two findings the author did not Measured on station02 (loadavg 0.97–2.05, 20 cores, bun 1.3.14 / node v22.22.3), Claims re-derivedCLAIM 1 — the refuting control HOLDS, and it is the strongest thing in the PR.
Author reported 636.1 ms / 0.5 ms. Fewer occurrences, 1273x faster. The CLAIM 2 — both axes reproduce. 277x at 128 KiB — the PR's figure exactly. Newline-separated control: 2.78 ms Leak axis — I did not reuse the 22-shape corpus, I built an independent one. Zero drift on 40k well-formed inputs independently confirms the zero-drift claim CLAIM 3 — the negative control genuinely fails against shipped code. Copying Gates on the PR tree with a real THE RELEASE QUESTION — the regex fix IS sufficient on its own. Do not gate on bounding the call sites.Post-fix cost is linear all the way out, measured:
I also closed a gap the author flagged as unchecked. The PR fixes one rule; I Bounding the MCP input is worth doing as defence in depth, separately. It is not FindingsF1 (P2, non-blocking) — the code comment's absolute equivalence claim is false, and I can show the inputs. The comment asserts "no input that was redacted before stops being redacted" and "over-masking … is the cheap direction of that trade". Over-masking is not always the safe direction here, because the downstream rules key off parameter-shaped text. Over 60,000 randomised malformed header inputs: Minimal shape: F2 (P2, non-blocking) — the suite has ZERO discriminating coverage for the rule this PR rewrites. The author disclosed that the far-header test does not discriminate against a bounded variant. It is worse than disclosed: delete the Digest rule from F3 (P2) — the coverage argument for choosing the restructure over the simple cap is undemonstrated, and the cap is behaviourally inert. F4 (P3, not a PR defect) — "44 seconds at 512 KiB" does not reproduce and is not in this PR. I measured 10,158 ms at 512 KiB, which is exactly what 636 ms at 128 KiB predicts under the 3.97x/doubling everyone agrees on. The record carries three mutually inconsistent absolutes: 29,058 ms (sabinus, todos F5 (P3) — merging does not ship. The PR touches only Base drift (informational): PR is based on Correctly out of scope, verified: the escaped-quote JSON leak. I measured it independently — What I did NOT check
Verdict: GO. The ReDoS is closed, the complexity class is fixed pipeline-wide, and the equivalence claim survives a corpus 1800x larger than the one it was made on. F1's comment correction and F2's test guard should land as follow-ups; neither blocks merge. F5 is what actually protects users. |
Closes the quadratic in the
response=redaction rule shipped in@hasna/tai0.1.3.Refs todos
a0b7904f. Code only — not the docs correction, which is #16.What is wrong
The lazy
[^\r\n]*?rescans to end-of-line from every position whereauthorization...Digestmatches. Many matches on one line is O(n²). A newlinebounds each rescan, which is why ordinary multi-line log text stayed linear and
this never showed up in normal use.
It was introduced by the security release itself — 0.1.2 does not have it.
The mechanism, not just the curve
The cost is the scan distance, not the number of matches. Control, on one
128 KiB line each, median of 5:
DigestoccurrencesAuthorization: Digest)Authorization: Digest response=x)Fewer occurrences would predict faster, and the occurrence-count hypothesis
predicts the opposite of what was measured. The scan-distance hypothesis is the
one that survives.
Reachability — why this is a ReDoS and not a slow function
src/agentic.ts:97-98,105-106—redactSensitiveText(stdout).slice(0, 12000).The slice runs after redaction, so the 12k truncation bounds nothing. The
real bound is
maxBuffer: 128 * 1024.src/mcp/index.ts:107— caller-supplied MCP tool text, nomaxBufferat all.Single-threaded runtime, so this blocks the event loop.
Scope of live exposure, stated in both directions. On the Hasna fleet this is
currently latent: nothing runs
tai(pgrep -x tai-mcp= 0, against a controlof
pgrep -x bash= 89, so the probe works). Public consumers who installed0.1.3 from npm are exposed now and are outside any sweep we can run. The rung
that bounded this — INSTALLED is not RUNNING — is the same rung that lets a
process already running keep executing pre-fix bytes after a fix ships. It cuts
both ways and is not a formality in either direction.
Both axes, measured separately
A zero-leak result and a flat timing curve are two different measurements.
0.1.3 was verified at every rung — registry, dist-tag, installed version,
in-bundle, with controls at each — and not one of those rungs could have seen a
quadratic. Every rung verified, no axis for size. So both are recorded here.
Axis 1 — does it leak
22-shape A/B corpus, pre-fix (
fc8bc4d, shipped as 0.1.3) vs post-fix,compared byte-for-byte:
Zero output drift — every shape produces byte-identical output before and
after. The speedup was not bought by matching less.
Three controls in the same run, so the leak count cannot be vacuous:
Cookie: session=…)The
before=1 / after=1leak is a pre-existing JSON-escaped-quote gap thatthis PR does not touch and does not worsen:
{"headers":{"authorization":"Digest username=\"u\", response=\"<cred>\""}}leaks identically in both versions,because the quoted alternative stops at the first escaped
\". Filed separately;it is the same class as todos
83702b35oniapp-sms. Fixing it here would havemade this PR's leak-count evidence unreadable.
Axis 2 — how does it scale
station02, loadavg 1.06–1.25, median of 5, repeated
Authorization: Digestonone line:
{0,256}?Paired control at the same byte count, newline-separated (post-fix): 3.0 ms at
128 KiB, 1.95x–2.00x. A third control — the same shape with
Basicinstead ofDigest— stays linear pre-fix, which isolates the defect to this rule ratherthan to the Authorization family.
Why a restructure and not a bound
I measured the bounded form rather than reasoning about it, and it is linear
— 1.99x per doubling. An earlier draft of my own comment asserted it "stays
O(n·N)"; that was wrong and the file now records the measurement instead of the
prediction.
The bound loses on the other axis. A cap declines to look past N, and a
Digest header carries
nonce,uri,opaqueandcnonceahead ofresponse,none bounded by the RFC — so any N small enough to be comfortable is an N past
which this rule stops looking. On the shapes tested the broader Authorization
rule masks such a header anyway, but that makes a leak rule depend on a
different rule as its safety net, and the two are edited separately.
The restructure carries no ceiling: the header value is captured whole
(
[^\r\n]*with nothing after it, so it never backtracks) and, because the matchconsumes to end-of-line, every later
authorization...Digeston that linefalls inside it and is never retried as a fresh start position.
response=ismasked inside that one captured value. It is also ~2x faster than the bounded
form.
Tests
Assert a growth ratio, not a millisecond figure — an absolute threshold turns
machine load into a test result, while the exponent does not move with load.
Negative control: against the shipped code the new test fails at 3.96x on
station01 at loadavg 14 — a contended box — so it discriminates rather than
passing on whatever it is pointed at.
The newline-separated control is committed alongside it as a pair: if a future
change reintroduces end-of-line rescanning the two diverge, which a single timing
could not distinguish from a busy machine.
What I did NOT check
response=sitting far into a long header is pinned, but it does notdiscriminate. I wrote it believing it guarded against a bounded-scan fix; I
then measured a
{0,256}?variant passing it too, because a downstream rulemasks that header. The test now says so in its own comment rather than carrying
a claim it cannot back. The real equivalence evidence is the 22-shape A/B run.
preventing a leak — the broader Authorization rule rescued every case I built.
That is a question about whether the rule is load-bearing at all; it is out of
scope here and I did not pursue it.
redactSensitiveTextonly; patterns arenot exported and I did not measure them individually.
agentic.tsandmcp/index.tswere read for reachability but are unchangedby this PR — the missing
maxBufferon the MCP path is still missing.Gates
Exit codes measured unpiped. Staged secrets scan run before the commit, clean.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.