Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions FIDELITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-07 (round 166) — a script-built DOM tree far deeper than any real page crashed every whole-document tree walk with an unrecoverable stack overflow, not a panic (security audit, patch)

Continuing this session's "audit de sécurité" pass after the rotate-crash fix below: checked for ReDoS and stack-overflow risk from pathological input. Go's `regexp` package is RE2-based (confirmed: every custom pattern in this repo — `svg.go`, `css/supports.go`, `css/selector.go`, `css/container.go`, `css/parse.go` — compiles under `regexp.MustCompile`, never a backtracking engine), so ReDoS is structurally impossible here, same category of finding as round 161's XXE check. Stack-overflow risk was real.

- **Finding**: `golang.org/x/net/html` already refuses to *parse* markup nested past 512 levels (`html: open stack of elements exceeds 512 nodes`), so a page's raw HTML can never build a pathologically deep tree. But that guard lives in the parser alone — `dom.AppendChild`/`InsertBefore` have no depth check of their own, by design (they are generic tree-mutation primitives, not a parser), so a page's own `<script>` can still build an arbitrarily deep tree at runtime (`document.createElement` + `appendChild` in a loop). Built a 500,000-deep tree directly in Go (bypassing both the HTML parser's cap and the 5-second JS script budget, to isolate the question precisely) and ran it through `css.Cascade` → `layout.LayoutDocumentViewport` → `paint.Paint`: it died with `fatal error: stack overflow` inside `css.CascadeMediaContainers`'s own recursive walk, somewhere between 150,000 and 200,000 levels. Unlike a panic, a Go stack overflow is **not recoverable** — `recover()` cannot catch it, so it kills the whole embedding process, not just the one render. Four separate unbounded recursive walks were found and fixed: `css.StylesheetLinks` (the very first DOM walk in the whole render pipeline), `collectAuthorRulesFrom`'s `styleElementText`, `css.Cascade`'s own element walk, and `layout`'s `place`/`contents` mutual recursion (paint's own recursion over the already-built `layout.Box` tree needs no separate cap: it is naturally bounded once layout's is in place, since paint only ever walks `Box.Children`, which layout's cap already keeps shallow).
- **Fixed**: each of the four walks now carries (or, for `layouter.place`, already carried as a natural per-call-site value) a depth counter, capped at 512 — reusing the HTML parser's own empirically-confirmed bound, which is both consistent with it and enormously generous relative to the measured 150,000–200,000 crash threshold. Past the cap, a walk simply stops descending (the subtree is left unstyled / un-laid-out, as if it were not there) rather than erroring — the same "cap and move on" shape this codebase already uses for `@import` cycles (`maxImportDepth`, `engine.go`) and `var()` cycles (`maxVarDepth`, `css/vars.go`). `layout.place` returns a minimal childless box past its own cap, using the same non-nil default style every `Box` already carries when passed a nil `Style`.
- **Test**: `TestCascadeDoesNotCrashOnAPathologicallyDeepTree` and `TestStylesheetLinksDoesNotCrashOnAPathologicallyDeepTree` (`css`), `TestLayoutDoesNotCrashOnAPathologicallyDeepTree` (`layout`) — each hand-builds a 10,000-level chain (bypassing `dom.Parse`, which would refuse it at 512 anyway) and confirms the walk returns instead of crashing. The layout test needed a hand-built `StyleMap` giving every node `display:block`, not `css.Cascade`'s own output: `Cascade` has the same cap, so its StyleMap would leave everything past ~512 unstyled, and `layout.hasBlockLevelChild` treats a nil style as "not block-level" — stopping the recursion there for a different reason, before layout's own cap ever needed to act. Building the map by hand isolates the two caps so each is exercises by its own test, not accidentally covered by the other one firing first.
- **Coverage**: `layout` initially dropped to 99.9% (the new cap branch in `place` unreached) then 100.0% once the `StyleMap` was fixed to actually drive recursion that deep (the first attempt, with an empty `StyleMap`, never recursed past the root at all — `hasBlockLevelChild` treated every unstyled child as inline). `css` is unaffected: its pre-existing 99.5% floor (several unrelated functions in `selector.go`, `background.go`, `calc.go`, `gradient_sample.go`, `parse.go`) predates this change, confirmed identical before and after via `git stash`.

## 2026-10-07 (round 165) — a 500-page bench sweep found a real renderer crash: `transform:rotate()` on a box with an overflowing descendant wrote past its own offscreen buffer (engine#263, patch)

Building a 500-page corpus (268 distinct sites, at most four pages each, round 164's own planned expansion) and running the bench against it crashed on page 21: `https://www.smashingmagazine.com/2018/10/mobile-app-retention-rate/`, with `panic: runtime error: index out of range [6339531] with length 6336512` inside `blendPixel`, reached via `blitImage` → `paintItem` → `paintRotated`. Reproduced directly with `cmd/render` against the live URL, no bench harness involved — a real, user-facing crash, not a test artefact.
Expand Down
45 changes: 35 additions & 10 deletions css/cascade.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,29 @@ func CascadeMediaContainers(root *dom.Node, m Media, externalSheets []string, co
// stays docRules throughout).
docRules := rules
docIdx := buildRuleIndex(docRules)
var walk func(n *dom.Node, parent Style, containerStack []containerFrame, rules []Rule, idx *ruleIndex, host *dom.Node)
walk = func(n *dom.Node, parent Style, containerStack []containerFrame, rules []Rule, idx *ruleIndex, host *dom.Node) {
var walk func(n *dom.Node, parent Style, containerStack []containerFrame, rules []Rule, idx *ruleIndex, host *dom.Node, depth int)
walk = func(n *dom.Node, parent Style, containerStack []containerFrame, rules []Rule, idx *ruleIndex, host *dom.Node, depth int) {
if depth > maxTreeDepth {
// A page's real DOM is never this deep; the only way to reach it is a
// script building a pathological chain (document.createElement +
// appendChild in a loop), which bypasses the HTML parser's own
// "open stack of elements exceeds 512 nodes" guard entirely (that
// guard only applies to parsed markup — see dom.AppendChild, which
// has no depth check of its own). Confirmed live: a synthetic
// 200000-deep tree crashes this walk with an unrecoverable "fatal
// error: stack overflow" (not a panic — recover() cannot catch it,
// so it takes down the whole embedding process, not just this
// render) well before reaching that depth. Leaving n and its
// subtree unstyled here is a deliberate, cheap truncation — layout
// already treats a nil style as an anonymous block box (see
// layouter.place), and layout's own matching cap (maxLayoutDepth)
// stops it from ever walking this deep into node.Children anyway.
return
}
if n.Type != dom.Element {
// Text/Document nodes have no computed style; recurse with parent.
for _, c := range n.Children {
walk(c, parent, containerStack, rules, idx, host)
walk(c, parent, containerStack, rules, idx, host, depth+1)
}
return
}
Expand Down Expand Up @@ -143,7 +160,7 @@ func CascadeMediaContainers(root *dom.Node, m Media, externalSheets []string, co
// is, correctly per spec, computed via the SAME outer scope as any
// other light-DOM element, never restyled by the shadow's rules).
for _, c := range n.Children {
walk(c, st, childStack, rules, idx, host)
walk(c, st, childStack, rules, idx, host, depth+1)
}
// n's shadow tree, if it has one: a NEW scope (shadowRules, host=n),
// plus any "::part(name)" selectors from n's OWN ambient scope
Expand All @@ -153,12 +170,12 @@ func CascadeMediaContainers(root *dom.Node, m Media, externalSheets []string, co
if n.Shadow != nil {
shadowScope := append(append([]Rule{}, shadowRules...), filterPartSelectors(rules)...)
for _, c := range n.Shadow.Children {
walk(c, st, childStack, shadowScope, nil, n)
walk(c, st, childStack, shadowScope, nil, n, depth+1)
}
}
}
// The synthetic Document root has no style; seed children with initial.
walk(root, initialStyle(), nil, docRules, docIdx, nil)
walk(root, initialStyle(), nil, docRules, docIdx, nil, 0)
return sm
}

Expand Down Expand Up @@ -372,8 +389,16 @@ func collectAuthorRulesFrom(nodes []*dom.Node, m Media) []Rule {
// context (DocumentPage, page.go) read it.
func styleElementText(nodes []*dom.Node) string {
var sb strings.Builder
var walk func(n *dom.Node)
walk = func(n *dom.Node) {
var walk func(n *dom.Node, depth int)
walk = func(n *dom.Node, depth int) {
if depth > maxTreeDepth {
// See the main cascade walk's own depth check for why this bound
// exists: this walk runs BEFORE it (collectAuthorRules is called
// first, to build the rules the main walk then matches), so without
// its own cap it would be the first of the two to crash on a
// pathologically deep, script-built tree.
return
}
if n.Type == dom.Element && n.Tag == "style" {
for _, c := range n.Children {
if c.Type == dom.Text {
Expand All @@ -383,11 +408,11 @@ func styleElementText(nodes []*dom.Node) string {
}
}
for _, c := range n.Children {
walk(c)
walk(c, depth+1)
}
}
for _, n := range nodes {
walk(n)
walk(n, 0)
}
return sb.String()
}
Expand Down
33 changes: 33 additions & 0 deletions css/cascade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,39 @@ func TestCascadeMarginBlockInlineShorthand(t *testing.T) {
}
}

func TestCascadeDoesNotCrashOnAPathologicallyDeepTree(t *testing.T) {
// A page's HTML can never nest this deep — golang.org/x/net/html already
// refuses to parse markup past 512 levels — but a script building a chain
// at runtime (document.createElement + appendChild in a loop) bypasses
// that parser-only guard entirely (dom.AppendChild has no depth check of
// its own). Before maxTreeDepth existed, this crashed BOTH of Cascade's
// own tree walks (styleElementText, called first via
// collectAuthorRules, and the main element walk) with an unrecoverable
// "fatal error: stack overflow" (not a panic — recover() cannot catch
// it) somewhere between 150,000 and 200,000 levels; this uses 10,000,
// comfortably past maxTreeDepth(512) while staying fast to build and run.
const depth = 10000
root := &dom.Node{Type: dom.Document}
html := &dom.Node{Type: dom.Element, Tag: "html", Attr: map[string]string{}}
root.Children = []*dom.Node{html}
html.Parent = root
cur := html
for i := 0; i < depth; i++ {
d := &dom.Node{Type: dom.Element, Tag: "div", Attr: map[string]string{}}
d.Parent = cur
cur.Children = append(cur.Children, d)
cur = d
}
sm := Cascade(root)
// Styling stops at maxTreeDepth: the root Document (depth 0, never
// itself styled) recurses to html at depth 1, so exactly maxTreeDepth
// elements (html plus maxTreeDepth-1 divs) get styled before the walk
// gives up on everything past that.
if len(sm) != maxTreeDepth {
t.Fatalf("styled %d nodes, want %d (maxTreeDepth)", len(sm), maxTreeDepth)
}
}

// TestCascadePaddingLogicalProperties covers padding-block-start/end,
// padding-inline-start/end, and the padding-block/padding-inline shorthands
// — the padding-side sibling of the margin logical-property fix above.
Expand Down
15 changes: 11 additions & 4 deletions css/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,25 @@ type LinkRef struct {
// that carry the "alternate" token, or that have an empty href, are skipped.
func StylesheetLinks(root *dom.Node) []LinkRef {
var refs []LinkRef
var walk func(n *dom.Node)
walk = func(n *dom.Node) {
var walk func(n *dom.Node, depth int)
walk = func(n *dom.Node, depth int) {
if depth > maxTreeDepth {
// See cascade.go's own depth check: this is the first DOM walk in
// the render pipeline (engine.go calls it before any cascade), so
// without this cap it would be the first to crash on a
// pathologically deep, script-built tree.
return
}
if n.Type == dom.Element && n.Tag == "link" {
if ref, ok := linkRef(n); ok {
refs = append(refs, ref)
}
}
for _, c := range n.Children {
walk(c)
walk(c, depth+1)
}
}
walk(root)
walk(root, 0)
return refs
}

Expand Down
35 changes: 35 additions & 0 deletions css/external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,38 @@ func TestImportURLsSingleQuoteUnterminated(t *testing.T) {
t.Fatalf("want nil for unterminated quote, got %#v", u)
}
}

func TestStylesheetLinksDoesNotCrashOnAPathologicallyDeepTree(t *testing.T) {
// See TestCascadeDoesNotCrashOnAPathologicallyDeepTree (cascade_test.go)
// for the full rationale: a script-built chain (document.createElement +
// appendChild in a loop) can nest far past what golang.org/x/net/html
// would ever parse, and this walk — the first one to run in the whole
// render pipeline, called before any cascade — needs its own maxTreeDepth
// cap for the same reason.
const depth = 10000
root := &dom.Node{Type: dom.Document}
html := &dom.Node{Type: dom.Element, Tag: "html", Attr: map[string]string{}}
root.Children = []*dom.Node{html}
html.Parent = root
cur := html
for i := 0; i < depth; i++ {
d := &dom.Node{Type: dom.Element, Tag: "div", Attr: map[string]string{}}
d.Parent = cur
cur.Children = append(cur.Children, d)
cur = d
}
// One <link> within the cap, one past it: only the first is found.
within := &dom.Node{Type: dom.Element, Tag: "link",
Attr: map[string]string{"rel": "stylesheet", "href": "within.css"}}
within.Parent = html
html.Children = append(html.Children, within)
beyond := &dom.Node{Type: dom.Element, Tag: "link",
Attr: map[string]string{"rel": "stylesheet", "href": "beyond.css"}}
beyond.Parent = cur
cur.Children = append(cur.Children, beyond)

got := StylesheetLinks(root)
if len(got) != 1 || got[0].Href != "within.css" {
t.Fatalf("StylesheetLinks = %#v, want only within.css", got)
}
}
15 changes: 15 additions & 0 deletions css/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,21 @@ import "strings"
// var() chains are only a few levels deep.
const maxVarDepth = 64

// maxTreeDepth bounds DOM-tree recursion depth in every whole-document walk in
// this package (StylesheetLinks, collectAuthorRules's styleElementText, and
// Cascade's own element walk). golang.org/x/net/html already refuses to PARSE
// markup nested past 512 levels ("html: open stack of elements exceeds 512
// nodes"), but that guard lives in the parser alone: a page's script can still
// build an arbitrarily deep tree at runtime (document.createElement +
// appendChild in a loop — dom.AppendChild has no depth check of its own, by
// design: it is a generic tree-mutation primitive, not a parser). Confirmed
// live: an unguarded recursive walk over such a tree hits an unrecoverable
// "fatal error: stack overflow" (not a panic; recover() cannot catch it, so it
// kills the whole embedding process) somewhere between 150,000 and 200,000
// levels — reusing the parser's own 512 bound here is both consistent with it
// and enormously generous relative to that measured threshold.
const maxTreeDepth = 512

// isCustomProperty reports whether a property name is a CSS custom property
// (its name starts with two dashes, e.g. "--main-color"). Custom-property names
// are case-sensitive, unlike normal property names.
Expand Down
29 changes: 29 additions & 0 deletions layout/layout.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ type layouter struct {
// any downstream painter, know which piece of which <span> a word belongs
// to. Nil for ordinary text, and reset at each inline formatting context.
decor []inlineDecor

// depth counts nested place calls — every recursive box placement in this
// package (plain block/inline children, flex items, grid items, floats,
// multicol columns, inline-block/flex descendants) ultimately calls
// l.place for each child box, so incrementing it there alone (see place's
// own doc comment) bounds total Go call-stack depth regardless of which
// container algorithm is driving the recursion at any given level.
depth int
}

// beginInlineContext resets the whitespace-collapsing and pending-margin state
Expand Down Expand Up @@ -196,15 +204,36 @@ func firstElement(n *dom.Node) *dom.Node {
return nil
}

// maxLayoutDepth bounds place's own recursion depth — see css.maxTreeDepth
// (the equivalent, independently-enforced bound in the cascade walk that runs
// before layout) for the full rationale and the measured crash threshold this
// reuses the same generous constant against: a script can build a DOM tree far
// deeper than any real page (document.createElement + appendChild in a loop,
// which bypasses the HTML parser's own 512-node depth limit entirely), and
// this package's own place/contents mutual recursion walks node.Children
// directly — independently of however deep css.Cascade's StyleMap actually
// reaches — so it needs its own cap to avoid the same unrecoverable "fatal
// error: stack overflow" even when the StyleMap feeding it was already
// truncated.
const maxLayoutDepth = 512

// place lays out one block-level box within a containing block whose content
// origin x is cx and content width cw, advancing the block formatting context b.
func (l *layouter) place(node *dom.Node, st *css.Style, cx, cw float64, b *bfc) *Box {
l.depth++
defer func() { l.depth-- }()
if st == nil {
s := css.Style{Display: css.DisplayBlock, Width: css.Length{Auto: true},
MinWidth: css.Length{Auto: true}, MaxWidth: css.Length{Auto: true},
Height: css.Length{Auto: true}, ColumnWidth: css.Length{Auto: true}}
st = &s
}
if l.depth > maxLayoutDepth {
// Beyond the cap, stop descending: return a minimal, childless box
// (with the same non-nil, fully-populated style every Box carries,
// computed just above) rather than recursing into contents.
return &Box{Node: node, Style: st}
}
box := &Box{Node: node, Style: st}

bw := st.Border.Widths()
Expand Down
40 changes: 40 additions & 0 deletions layout/layout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,46 @@ func TestEmptyDocument(t *testing.T) {
}
}

func TestLayoutDoesNotCrashOnAPathologicallyDeepTree(t *testing.T) {
// A page's HTML can never nest this deep — golang.org/x/net/html already
// refuses to parse markup past 512 levels — but a script building a chain
// at runtime (document.createElement + appendChild in a loop) bypasses
// that parser-only guard entirely (dom.AppendChild has no depth check of
// its own). Before maxLayoutDepth existed, place's own recursion into
// contents crashed with an unrecoverable "fatal error: stack overflow"
// (not a panic — recover() cannot catch it) somewhere between 150,000 and
// 200,000 levels; this uses 10,000, comfortably past maxLayoutDepth(512)
// while staying fast to build and run.
const depth = 10000
root := &dom.Node{Type: dom.Document}
html := &dom.Node{Type: dom.Element, Tag: "html", Attr: map[string]string{}}
root.Children = []*dom.Node{html}
html.Parent = root
// A real StyleMap entry per node, not css.Cascade's own output: Cascade
// has its own, independently-enforced depth cap (css.maxTreeDepth, same
// value) that would otherwise leave every node past ~512 unstyled —
// hasBlockLevelChild treats a nil style as "not block-level" and stops
// recursing right there, which would stop this test from ever reaching
// deep enough to need LAYOUT's own cap at all. Building the map by hand
// isolates the two caps so each is actually exercised by its own test.
blockStyle := &css.Style{Display: css.DisplayBlock, Width: css.Length{Auto: true},
MinWidth: css.Length{Auto: true}, MaxWidth: css.Length{Auto: true},
Height: css.Length{Auto: true}, ColumnWidth: css.Length{Auto: true}}
sm := css.StyleMap{html: blockStyle}
cur := html
for i := 0; i < depth; i++ {
d := &dom.Node{Type: dom.Element, Tag: "div", Attr: map[string]string{}}
d.Parent = cur
cur.Children = append(cur.Children, d)
sm[d] = blockStyle
cur = d
}
box, _ := LayoutDocument(root, sm, 1024, fakeMeasurer{}, nil)
if box == nil {
t.Fatal("LayoutDocument returned a nil box")
}
}

func TestAttrFloat(t *testing.T) {
el := &dom.Node{Type: dom.Element, Tag: "img", Attr: map[string]string{
"width": "12px", "bad": "xyz",
Expand Down
Loading