diff --git a/FIDELITY.md b/FIDELITY.md index 510f0c7..eb8a852 100644 --- a/FIDELITY.md +++ b/FIDELITY.md @@ -18,6 +18,15 @@ The committed PNGs under `testdata/renders/` back every claim here. Reproduce them with the commands at the bottom. The measured-vs-Chrome numbers live in [`bench/REPORT.md`](bench/REPORT.md). +## 2026-10-11 (round 170) — a pathologically deep `calc()` nesting in a single `style="..."` attribute crashed the process with an unrecoverable stack overflow, the same class of bug round 166 fixed for DOM trees (security audit, patch) + +Continuing the "audit de sécurité" pass after round 166's DOM-depth fix: checked whether any OTHER recursive-descent parser in this codebase has the same shape — unbounded recursion keyed to attacker-controlled input depth, with no equivalent to the HTML parser's own 512-node guard. `css/calc.go`'s `calcParser` does: its grammar is `expr := term (('+'|'-') term)*`, `term := factor (('*'|'/') factor)*`, `factor := number[unit] | '(' expr ')'` — a textbook mutually-recursive expression grammar, and `factor`'s own paren branch recurses straight back into `expr` with no depth check at all. + +- **Finding**: unlike the DOM-depth case, nothing upstream bounds how many parens a single `calc(...)` value's SOURCE TEXT can carry — a `style="width:calc((((...1...))))"` attribute on the main document (itself capped only at 32MB — see `engine.go`'s own body-size cap) can carry millions of them without ever touching a separate parser-level guard the way deep HTML *element* nesting does (x/net/html's own 512-node limit has no equivalent for a single attribute VALUE's own text). Confirmed live: a synthetic 3,000,000-deep `calc((((...1...))))` crashes with `fatal error: stack overflow` (not a panic — `recover()` cannot catch it) where 1,000,000 does not yet. +- **Fixed**: `calcParser` now carries a `depth` counter, incremented/decremented around each paren group in `factor`, capped at `maxCalcDepth` (256 — enormously generous against the measured 1,000,000–3,000,000 crash threshold, and far past anything a real `calc()` expression ever nests in practice). Past the cap, `factor` returns its own existing "unparseable" failure (`0, false, false`) — the exact same degradation this parser already uses for any other malformed input — rather than recursing further. +- **Test**: `TestResolveCalcDoesNotCrashOnPathologicallyDeepParens` — a 10,000-deep synthetic value (comfortably past the 256 cap, cheap to build and run) asserts `resolveCalc` reports failure instead of crashing. +- **Coverage**: `css` package coverage unaffected (99.5% before and after, confirmed via `git stash`); `factor`'s own coverage actually rises slightly (92.0%→95.0%) since the new depth-cap branch is exercised by the new test — the two statements it remains short of (a `ParseFloat` failure path, a numeric-sign-prefix path) are pre-existing gaps this round did not touch. + ## 2026-10-11 (round 169) — go.mod's own declared Go version had drifted behind CI's, making `govulncheck` report 8 already-patched stdlib vulnerabilities as live; `golang.org/x/net` bumped too (security audit, patch) A concurrent session's PR #270 moved CI's `GO_VERSION` from 1.27.1 to 1.27.2 (a security release, 13 stdlib advisories) but — correctly, since a `go` directive is a floor, not a pin — left `go.mod`'s own `go 1.27.1` line untouched. Re-reading this session's own notes per the standing "relis tes notes" instruction, then re-running `govulncheck` to confirm the new CI pin actually closed something, surfaced a real gap: `govulncheck ./...` reported 8 reachable stdlib vulnerabilities (GO-2026-6605, -6603, -6607, -6608, and others) "Found in: net/http@go1.27.1, Fixed in: go1.27.2" — even though the installed toolchain is 1.27.2. `govulncheck -version` showed why: it had resolved its own subprocess `go` to literally **1.27.1**, not the 1.27.2 my interactive shell's `go version` reported — a live, concrete instance of the exact trap an earlier round's own memory note already names ("Go's `stable` tag can silently point at a different minor than you think — measure with the CI's own toolchain, not an assumption"), except here the drift was go.mod's own stale `go` directive feeding govulncheck's stdlib-vulnerability baseline, not a `stable`-tag alias. diff --git a/css/calc.go b/css/calc.go index 8324f07..18d3165 100644 --- a/css/calc.go +++ b/css/calc.go @@ -102,8 +102,24 @@ func evalCalcExpr(s string) (val float64, hasUnit bool, ok bool) { type calcParser struct { s string i int + // depth counts nested '(' ... ')' groups — factor's own recursion into + // expr for a parenthesized sub-expression (via term) is the only cycle + // in this grammar, so bounding it there bounds total Go call-stack + // depth. A value's own source text is this parser's only input, and + // nothing upstream limits how many parens it can carry (a style="..." + // attribute on the main document, itself capped only at 32MB — see + // engine.go's own body-size cap — can trivially carry millions): + // confirmed live, a 3,000,000-deep calc((((...1...)))) crashes with an + // unrecoverable "fatal error: stack overflow" (not a panic — recover() + // cannot catch it) where 1,000,000 does not yet. maxCalcDepth is + // enormously generous against that measured threshold while being far + // past anything a real calc() expression ever nests (a handful of + // levels, in practice). + depth int } +const maxCalcDepth = 256 + func (p *calcParser) skipSpace() { for p.i < len(p.s) && (p.s[p.i] == ' ' || p.s[p.i] == '\t' || p.s[p.i] == '\n' || p.s[p.i] == '\r') { p.i++ @@ -182,6 +198,14 @@ func (p *calcParser) term() (float64, bool, bool) { func (p *calcParser) factor() (float64, bool, bool) { p.skipSpace() if p.i < len(p.s) && p.s[p.i] == '(' { + if p.depth >= maxCalcDepth { + // Past the cap, treat this calc() as unparseable — the same + // failure this parser already returns for any other malformed + // input — rather than recursing further into expr. + return 0, false, false + } + p.depth++ + defer func() { p.depth-- }() p.i++ v, u, ok := p.expr() if !ok { diff --git a/css/calc_test.go b/css/calc_test.go index d4f96fb..c11b52f 100644 --- a/css/calc_test.go +++ b/css/calc_test.go @@ -3,7 +3,10 @@ package css -import "testing" +import ( + "strings" + "testing" +) func TestResolveCalcNoOccurrence(t *testing.T) { got, ok := resolveCalc("1px solid red") @@ -50,6 +53,21 @@ func TestResolveCalcNestedParens(t *testing.T) { } } +func TestResolveCalcDoesNotCrashOnPathologicallyDeepParens(t *testing.T) { + // A style="..." attribute on the main document (itself capped only at + // 32MB, see engine.go) can trivially carry a calc() value with millions + // of nested parens. Before maxCalcDepth existed, factor's own recursion + // into expr for each '(' crashed with an unrecoverable "fatal error: + // stack overflow" (not a panic — recover() cannot catch it) somewhere + // between 1,000,000 and 3,000,000 levels; this uses 10,000, comfortably + // past maxCalcDepth(256) while staying fast to build and run. + const depth = 10000 + value := "calc(" + strings.Repeat("(", depth) + "1" + strings.Repeat(")", depth) + ")" + if _, ok := resolveCalc(value); ok { + t.Error("want ok=false past the depth cap, got true") + } +} + func TestResolveCalcMultipleInOneValue(t *testing.T) { got, ok := resolveCalc("calc(1px * 2) solid calc(4px + 4px)") if !ok || got != "2px solid 8px" {