From a856411ac52aeb7e8e75b1311b7e1be55c544177 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 11 Oct 2026 18:23:09 +0200 Subject: [PATCH] security: cap calc() paren-nesting depth to stop a stack overflow calcParser's grammar (expr/term/factor, the standard mutually- recursive expression-parser shape) recurses from factor straight back into expr for each '(' with no depth check. Unlike deep HTML element nesting, nothing upstream bounds how many parens a single calc(...) value's own source text can carry -- a style="..." attribute on the main document (capped only at 32MB) can trivially carry millions. Confirmed live: a synthetic 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. factor now caps its own recursion at 256 levels, failing the same way this parser already fails on any other malformed input rather than recursing further. Co-Authored-By: Claude Sonnet 5 --- FIDELITY.md | 9 +++++++++ css/calc.go | 24 ++++++++++++++++++++++++ css/calc_test.go | 20 +++++++++++++++++++- 3 files changed, 52 insertions(+), 1 deletion(-) 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" {