security: cap cloneNode/isEqualNode recursion depth

FIDELITY.md

@@ -18,6 +18,16 @@ 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-12 (round 171) — `node.cloneNode()`/`node.isEqualNode()` recursed on a script-built tree independently of round 166's cascade/layout caps — a script reaches them directly, bypassing that pipeline entirely (security audit, patch)
+
+Continuing the same sweep as round 170, this time across JS-reachable DOM methods rather than CSS parsers: `js/dom.go`'s `isEqualNode` (structural comparison, `node.isEqualNode(other)`) and `cloneNode` (`node.cloneNode(deep)`, also `document.importNode`) each recurse over a node's own `Children` with no depth check — and critically, round 166's own fix (capping the CSS cascade and layout walks) does **not** cover this: those caps guard the pipeline a page's tree flows through automatically during rendering, while these two methods are called directly by a script on whatever tree it already has, independently of whether that tree is ever rendered at all.
+
+- **Finding**: confirmed with a 10,000-deep synthetic chain (built directly, bypassing `dom.Parse`'s 512-level refusal, the same `createElement`+`appendChild`-loop shape round 166 already established has no depth check of its own) that both functions recurse unboundedly. Checked two JS-reachable ways an attacker might build a tree deep enough to matter without excessive `createElement`/`appendChild` FFI overhead (a direct loop, and a `cloneNode`-and-reinsert doubling trick) — neither reached a dangerous depth within a reasonable script budget in practice, consistent with round 166's own finding that JS-driven tree construction is inherently throttled by call overhead. That made this lower urgency than round 166's or round 170's own findings, but the underlying gap (recursion with literally no bound, reachable in principle regardless of how hard it is to construct the input today) is the same category this session has treated as worth closing on principle: `recover()` cannot catch a Go stack overflow regardless of how the input arrived.
+- **Fixed**: both functions now take a `depth` parameter, capped at a new `maxDOMWalkDepth = 512` (matching the HTML parser's own bound, same reasoning as `css.maxTreeDepth`/`layout.maxLayoutDepth` from round 166). Past the cap: `isEqualNode` reports `false` (two trees too deep to safely compare are deemed unequal, never crashing); `cloneNode` stops descending and returns a real, correctly-typed node with an empty subtree rather than failing the whole call.
+- **Test**: `js/dom_test.go` (new file) — `TestIsEqualNodeDoesNotCrashOnPathologicallyDeepTree`, `TestIsEqualNodeDeepMismatchPastCapIsNotEqual`, `TestCloneNodeDoesNotCrashOnPathologicallyDeepTree`, each using a 10,000-deep synthetic chain.
+- **Coverage**: `js` package stays at its 95.9% baseline; the two uncovered statements in `isEqualNode` (an attribute-count mismatch, a children-count mismatch) are pre-existing gaps this round's depth-cap addition did not touch.
+- Also discovered and fixed in passing: the canonical ten-page `bench/REPORT.md`/`bench/results.json`/two montage PNGs had been silently overwritten with 500-page-corpus data at some earlier point in this session (the exact round-166 mistake recurring, likely from before that round's own "always pass distinct -report/-json/-out" fix, sitting unstaged and unnoticed through several subsequent commits since `git commit` without `-a` never touched it). Restored via `git stash` (recoverable, not discarded) rather than `git checkout --`.
+
 ## 2026-10-11 (round 170) — three text-level CSS parsers (`calc()`, `:not()`, `:is()`) recursed on attacker-controlled nesting depth with no guard, the same class of bug round 166 fixed for DOM trees — one of them exponential, not merely deep recursion (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.

js/dom.go

@@ -15,6 +15,18 @@ import (
 	"github.com/go-webengine/engine/dom"
 )
 
+// maxDOMWalkDepth bounds a few DOM methods' own recursion
+// (isEqualNode/cloneNode) that a script reaches directly, independently of
+// the cascade/layout pipeline round 166 already capped at the same value
+// (css.maxTreeDepth, layout.maxLayoutDepth). A page's own createElement+
+// appendChild loop can build a tree deeper than golang.org/x/net/html would
+// ever parse (dom.AppendChild has no depth check of its own, by design), and
+// calling one of these methods on such a tree would otherwise recurse with
+// no bound at all. 512 matches the other two caps for the same reason they
+// chose it: the HTML parser's own empirically-confirmed limit, enormously
+// generous against any real page while keeping the worst case trivial.
+const maxDOMWalkDepth = 512
+
 // accessor defines a JS accessor property on o. A nil set makes it read-only.
 func (b *binder) accessor(o *goja.Object, name string, get func() goja.Value, set func(goja.Value)) {
 	getter := b.vm.ToValue(func(goja.FunctionCall) goja.Value { return get() })
@@ -402,7 +414,7 @@ func (b *binder) defineElement(o *goja.Object, n *dom.Node) {
 		return b.vm.ToValue(contains(n, b.node(call.Argument(0))))
 	})
 	o.Set("isEqualNode", func(call goja.FunctionCall) goja.Value {
-		return b.vm.ToValue(isEqualNode(n, b.node(call.Argument(0))))
+		return b.vm.ToValue(isEqualNode(n, b.node(call.Argument(0)), 0))
 	})
 	o.Set("hasChildNodes", func(call goja.FunctionCall) goja.Value {
 		return b.vm.ToValue(len(n.Children) > 0)
@@ -1681,7 +1693,22 @@ func precedesInTreeOrder(root, a, b *dom.Node) bool {
 // already-documented reskin/orphaned-node gap (rounds 10/19): that washed-
 // out-prose symptom is still present, unchanged, after this fix. The two
 // are independent defects that happen to both live in the same settle path.
-func isEqualNode(a, b *dom.Node) bool {
+//
+// depth bounds this function's own recursion the same way round 166's
+// DOM-walk caps do (css.maxTreeDepth, layout.maxLayoutDepth) — but
+// independently of them: those guard the CASCADE/LAYOUT pipeline a page's
+// tree flows through automatically, while isEqualNode is a DOM method a
+// script calls directly (node.isEqualNode(other)), on ANY tree it can build
+// via createElement+appendChild (which, per those same rounds' own finding,
+// has no depth check of its own) — a separate call path round 166 never
+// covered. A root call (the JS binding below) always passes 0.
+func isEqualNode(a, b *dom.Node, depth int) bool {
+	if depth > maxDOMWalkDepth {
+		// Two trees too deep to safely compare are deemed unequal rather
+		// than crashing — the same "a result that degrades rather than
+		// matches" choice contains/cloneNode make past their own cap.
+		return false
+	}
 	if a == nil || b == nil {
 		return a == b
 	}
@@ -1700,7 +1727,7 @@ func isEqualNode(a, b *dom.Node) bool {
 		return false
 	}
 	for i, ca := range a.Children {
-		if !isEqualNode(ca, b.Children[i]) {
+		if !isEqualNode(ca, b.Children[i], depth+1) {
 			return false
 		}
 	}
@@ -1729,7 +1756,16 @@ func isEqualNode(a, b *dom.Node) bool {
 // element (as opposed to the far more common `tpl.content.cloneNode(true)`
 // idiom, which never went through this path since .content is itself a
 // plain fragment) would crash on the very next `.content` access.
+//
+// depth bounds this function's own recursion the same way isEqualNode's own
+// doc comment explains — a script calling `node.cloneNode(true)` reaches
+// this independently of the cascade/layout caps round 166 added, on any
+// tree it built itself. A root call (the JS bindings) always passes 0.
 func cloneNode(n *dom.Node, deep bool) *dom.Node {
+	return cloneNodeDepth(n, deep, 0)
+}
+
+func cloneNodeDepth(n *dom.Node, deep bool, depth int) *dom.Node {
 	if n == nil {
 		return nil
 	}
@@ -1740,17 +1776,24 @@ func cloneNode(n *dom.Node, deep bool) *dom.Node {
 			c.Attr[k] = v
 		}
 	}
+	// Past the cap, stop descending: the clone's subtree below this point is
+	// left empty rather than crashing — the only degradation that keeps the
+	// clone a real, usable node of the right type rather than failing the
+	// whole call.
+	if depth > maxDOMWalkDepth {
+		return c
+	}
 	if n.Tag == "template" {
 		c.Content = &dom.Node{Type: dom.Element, Tag: "#fragment", Attr: map[string]string{}}
 		if deep && n.Content != nil {
 			for _, ch := range n.Content.Children {
-				dom.AppendChild(c.Content, cloneNode(ch, true))
+				dom.AppendChild(c.Content, cloneNodeDepth(ch, true, depth+1))
 			}
 		}
 	}
 	if deep {
 		for _, ch := range n.Children {
-			dom.AppendChild(c, cloneNode(ch, true))
+			dom.AppendChild(c, cloneNodeDepth(ch, true, depth+1))
 		}
 	}
 	return c

js/dom_test.go

@@ -0,0 +1,64 @@
+// Copyright (c) the go-webengine/engine authors.
+// SPDX-License-Identifier: BSD-3-Clause
+
+package js
+
+import (
+	"testing"
+
+	"github.com/go-webengine/engine/dom"
+)
+
+// deepChain builds a depth-level-deep chain of div elements under a single
+// root, bypassing dom.Parse (which would refuse it past 512 levels) — the
+// same shape a page's own createElement+appendChild loop can build at
+// runtime, since dom.AppendChild has no depth check of its own.
+func deepChain(depth int) *dom.Node {
+	root := &dom.Node{Type: dom.Element, Tag: "div", Attr: map[string]string{}}
+	cur := root
+	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
+	}
+	return root
+}
+
+func TestIsEqualNodeDoesNotCrashOnPathologicallyDeepTree(t *testing.T) {
+	// A script calling node.isEqualNode(other) reaches this independently of
+	// the cascade/layout depth caps round 166 added — isEqualNode is never
+	// in that pipeline at all. Before maxDOMWalkDepth existed, this
+	// recursion had no bound; this uses 10,000, comfortably past
+	// maxDOMWalkDepth(512) while staying fast to build and run.
+	//
+	// The two chains are IDENTICAL in shape, which is deliberate: past the
+	// cap this function reports "not equal" unconditionally (see its own
+	// doc comment — a documented, intentional degradation, not a bug), so
+	// this specifically proves the cap-triggered path returns rather than
+	// crashes, not that equal deep trees stay reported as equal.
+	a := deepChain(10000)
+	b := deepChain(10000)
+	if isEqualNode(a, b, 0) {
+		t.Error("want false past the depth cap, even for identical trees")
+	}
+}
+
+func TestIsEqualNodeDeepMismatchPastCapIsNotEqual(t *testing.T) {
+	a := deepChain(10000)
+	b := deepChain(9999) // one level shallower
+	if isEqualNode(a, b, 0) {
+		t.Error("trees of different depth should never compare equal")
+	}
+}
+
+func TestCloneNodeDoesNotCrashOnPathologicallyDeepTree(t *testing.T) {
+	root := deepChain(10000)
+	clone := cloneNode(root, true)
+	if clone == nil {
+		t.Fatal("cloneNode returned nil")
+	}
+	if clone.Tag != root.Tag {
+		t.Errorf("clone.Tag = %q, want %q", clone.Tag, root.Tag)
+	}
+}