From fc1c6d9a8849048cb1309d77bffa46289c0caee4 Mon Sep 17 00:00:00 2001
From: tannevaled <tannevaled@users.noreply.github.com>
Date: Sat, 17 Oct 2026 23:04:59 +0200
Subject: [PATCH] security: cap total node count in convertChildren
 (Parse/ParseFragment)

Continuing round 172's own sweep into the one other place a script
routinely hands this engine a whole subtree at once: innerHTML/
outerHTML/insertAdjacentHTML, which all route through
dom.ParseFragment (or dom.Parse for the top-level page), sharing
one core: convertChildren.

golang.org/x/net/html's own parser refuses markup nested past 512
levels, but that guard is about depth -- a WIDE document (millions
of sibling elements, never nesting past a handful of levels)
sails straight through it untouched.

Confirmed live, two ways: `root.innerHTML =
"<div>x</div>".repeat(3000000)` -- one statement, no loop --
allocated ~11.5GB of real memory in 15 seconds; a 25-iteration
`root.innerHTML = root.outerHTML + root.outerHTML` loop (the same
doubling shape as round 172's cloneNode case, through
serialize-then-reparse instead of direct cloning) reached ~6GB.

convertChildren now carries a shared node-count budget
(maxParsedNodes=200000, matching js.maxCloneNodes's own value and
reasoning) threaded through its own recursion and the <template>
content-fragment branch. Past zero, remaining siblings/descendants
are simply not converted rather than crashing or exhausting
memory.

Measured effect: the flat repeat(3000000) case drops from
~11.5GB/15.4s to ~1.1GB/1.8s; the outerHTML-doubling loop drops
from ~6GB/8.3s to ~1.4GB/3.5s.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 FIDELITY.md        | 11 +++++++++++
 dom/dom.go         | 35 +++++++++++++++++++++++++++++++----
 dom/dom_test.go    | 24 +++++++++++++++++++++++-
 dom/mutate.go      |  3 ++-
 dom/mutate_test.go | 36 +++++++++++++++++++++++++++++++++++-
 5 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/FIDELITY.md b/FIDELITY.md
index 95502e5..fd60f9a 100644
--- a/FIDELITY.md
+++ b/FIDELITY.md
@@ -18,6 +18,17 @@ 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-17 (round 173) — the SAME memory-exhaustion shape as round 172, but simpler to trigger: a single `innerHTML` assignment, no loop at all, found by generalizing the lesson rather than re-checking the same code path (security audit, patch)
+
+Continuing round 172's own sweep (JS methods that can allocate unboundedly from a tree it already has) into the one other place a script routinely hands this engine a whole subtree at once: `innerHTML`/`outerHTML`/`insertAdjacentHTML`. All three ultimately call `dom.ParseFragment` (or, for the top-level page, `dom.Parse`), both of which share one core: `convertChildren`.
+
+- **Finding**: `golang.org/x/net/html`'s own parser refuses markup nested past 512 levels, but that guard is about DEPTH — a WIDE document (millions of sibling elements, never nesting past a handful of levels) sails straight through it. `convertChildren` had no count of its own at all. Confirmed live, two ways: `root.innerHTML = "<div>x</div>".repeat(3000000)` — one statement, no loop, the repeated fragment a mere 13 bytes — allocated **~11.5GB of real memory in 15 seconds**; a 25-iteration `root.innerHTML = root.outerHTML + root.outerHTML` loop (the same doubling shape as round 172's `cloneNode` case, but through serialize-then-reparse instead of direct cloning) reached ~6GB.
+- **Fixed**: `convertChildren` now carries a shared node-count `budget` (`maxParsedNodes = 200,000`, matching `js.maxCloneNodes`'s own value and reasoning — the same bug, one round earlier, for `cloneNode`'s bushy-growth case) threaded through both its own recursion and the `<template>` content-fragment branch. Past zero, remaining siblings/descendants are simply not converted — the real tree loses nodes near the bound rather than crashing or exhausting memory, the same degradation shape as every other budget cap in this codebase.
+- **Measured effect**: the flat `repeat(3000000)` reproduction drops from ~11.5GB/15.4s to ~1.1GB/1.8s; the `outerHTML`-doubling loop drops from ~6GB/8.3s to ~1.4GB/3.5s.
+- **Test**: `dom/mutate_test.go`'s `TestParseFragmentCapsTotalNodeCount` and `dom/dom_test.go`'s `TestParseCapsTotalNodeCount` (the shared `countTreeNodes` helper, new in `mutate_test.go`), each building a `maxParsedNodes+1000`-repeat string. `Parse`'s own count needed a `+1` margin over `ParseFragment`'s: `Parse` wraps `convertChildren`'s budgeted output in a synthetic `Document` node created BEFORE the budget starts counting, so the full tree is one node larger than the budget alone would suggest — confirmed precisely (it reported exactly `maxParsedNodes+1`, not a looser overshoot) before adjusting the assertion rather than second-guessing the fix.
+- **Coverage**: `convertChildren` at 100%; `dom` package 98.4%→98.5% (confirmed via `git stash`, no regression — and this time stashing tracked and untracked paths SEPARATELY, per round 172's own named lesson, which held up cleanly).
+- **Same honest caveat as round 172's own `cloneNode` fix**: this bounds one `Parse`/`ParseFragment` call, not a whole render — a script calling `innerHTML =` many times could still accumulate several×`maxParsedNodes` before the JS script timeout stops it. Bounded and linear now, not a hard per-render ceiling.
+
 ## 2026-10-17 (round 172) — closing round 171's own open lead: 25 lines of ordinary-looking script allocated ~21GB of REAL memory, and neither an existing heap watchdog nor this round's own depth cap could stop it (security audit, patch)
 
 Round 171 flagged, but did not characterize, a 25-iteration `root.appendChild(root.cloneNode(true))` loop that rendered a 10.5-million-pixel-tall page without crashing — unclear at the time whether that was genuinely safe (sparse, unpainted memory) or just not yet pushed hard enough. Measured directly with `runtime.MemStats` before/after: **~19.75GB of real HeapAlloc, ~21GB of real Sys**, in under 9 seconds, from 25 ordinary DOM calls on a trivial page. Not sparse at all — `image.NewRGBA` eagerly allocates and zero-fills every byte, and `newCanvas`'s own fill pass then writes to all of it.
diff --git a/dom/dom.go b/dom/dom.go
index 7e1c491..1569378 100644
--- a/dom/dom.go
+++ b/dom/dom.go
@@ -167,7 +167,8 @@ func Parse(htmlSrc string) (*Node, error) {
 		return nil, err
 	}
 	doc := &Node{Type: Document, Quirks: !hasDoctype(root)}
-	convertChildren(root, doc)
+	budget := maxParsedNodes
+	convertChildren(root, doc, &budget)
 	attachDeclarativeShadowRoots(doc)
 	return doc, nil
 }
@@ -184,14 +185,37 @@ func hasDoctype(root *html.Node) bool {
 	return false
 }
 
+// maxParsedNodes bounds the total nodes one Parse/ParseFragment call may
+// produce. golang.org/x/net/html already refuses markup nested past 512
+// levels, but that guard is about DEPTH, not total node count — a WIDE
+// document (millions of SIBLING elements, never nesting past a handful of
+// levels) sails through it untouched. Confirmed live: a single statement,
+// `root.innerHTML = "<div>x</div>".repeat(3000000)`, allocates ~11.5GB of
+// real memory in 15 seconds — no loop, no deep nesting, no large string by
+// pathological standards (the repeated fragment is 13 bytes). 200,000
+// matches js.maxCloneNodes (the same shape of bug, found one round
+// earlier, for cloneNode's own bushy-growth case) — generous for any real
+// page while keeping one parse's own worst case a bounded amount of memory.
+const maxParsedNodes = 200000
+
 // convertChildren walks h's children, converting element, text and comment
 // nodes and dropping everything else (doctype, processing instructions — no
 // real page's rendering depends on either surviving into the DOM). A
 // <template>'s own children are parsed into its Content fragment instead of
 // its Children, per spec (see Node.Content's doc comment) — this is the ONE
 // place besides NewElement a template can originate from, so both must agree.
-func convertChildren(h *html.Node, parent *Node) {
+//
+// budget bounds the TOTAL nodes this call (and its own recursion) may add —
+// see maxParsedNodes. Past zero, remaining siblings/descendants are simply
+// not converted (the real tree loses nodes near this bound rather than
+// crashing or exhausting memory) — the same "reduce, don't drop entirely"
+// shape as the budget caps elsewhere in this codebase (css's
+// expandFunctionalPseudosBudget, js's cloneNode).
+func convertChildren(h *html.Node, parent *Node, budget *int) {
 	for c := h.FirstChild; c != nil; c = c.NextSibling {
+		if *budget <= 0 {
+			return
+		}
 		switch c.Type {
 		case html.CommentNode:
 			parent.Children = append(parent.Children, &Node{
@@ -199,6 +223,7 @@ func convertChildren(h *html.Node, parent *Node) {
 				Text:   c.Data,
 				Parent: parent,
 			})
+			*budget--
 		case html.ElementNode:
 			tag := strings.ToLower(c.Data)
 			el := &Node{
@@ -211,11 +236,12 @@ func convertChildren(h *html.Node, parent *Node) {
 				el.Attr[strings.ToLower(a.Key)] = a.Val
 			}
 			parent.Children = append(parent.Children, el)
+			*budget--
 			if tag == "template" {
 				el.Content = &Node{Type: Element, Tag: "#fragment", Attr: map[string]string{}}
-				convertChildren(c, el.Content)
+				convertChildren(c, el.Content, budget)
 			} else {
-				convertChildren(c, el)
+				convertChildren(c, el, budget)
 			}
 		case html.TextNode:
 			// Preserve the raw text; whitespace handling is a layout concern.
@@ -224,6 +250,7 @@ func convertChildren(h *html.Node, parent *Node) {
 				Text:   c.Data,
 				Parent: parent,
 			})
+			*budget--
 		}
 	}
 }
diff --git a/dom/dom_test.go b/dom/dom_test.go
index 195cd55..aa09bab 100644
--- a/dom/dom_test.go
+++ b/dom/dom_test.go
@@ -3,7 +3,10 @@
 
 package dom
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestParseAndAttributes(t *testing.T) {
 	root, err := Parse(`<!DOCTYPE html><html><head><title> Hi </title></head>` +
@@ -151,3 +154,22 @@ func TestParseQuirksMode(t *testing.T) {
 		t.Error("<!DOCTYPE html> present: want Quirks=false")
 	}
 }
+
+func TestParseCapsTotalNodeCount(t *testing.T) {
+	// See TestParseFragmentCapsTotalNodeCount (mutate_test.go) for the full
+	// rationale: golang.org/x/net/html's own depth guard (512 levels) does
+	// nothing against a WIDE document, and this is the top-level Parse
+	// (the main document fetch) rather than ParseFragment, sharing the same
+	// convertChildren budget.
+	root, err := Parse(`<html><body>` + strings.Repeat("<div>x</div>", maxParsedNodes+1000) + `</body></html>`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// +1: root itself is the synthetic Document wrapper Parse creates BEFORE
+	// calling convertChildren, so it is one node outside the budget
+	// convertChildren's own recursion tracks (confirmed: without the +1,
+	// this reported exactly maxParsedNodes+1, not a looser overshoot).
+	if got := countTreeNodes([]*Node{root}); got > maxParsedNodes+1 {
+		t.Errorf("Parse produced %d nodes, want <= %d (maxParsedNodes+1)", got, maxParsedNodes+1)
+	}
+}
diff --git a/dom/mutate.go b/dom/mutate.go
index d487c91..4d2214d 100644
--- a/dom/mutate.go
+++ b/dom/mutate.go
@@ -263,7 +263,8 @@ func ParseFragment(htmlSrc string) ([]*Node, error) {
 		container.AppendChild(p)
 	}
 	holder := &Node{Type: Element, Tag: "body", Attr: map[string]string{}}
-	convertChildren(container, holder)
+	budget := maxParsedNodes
+	convertChildren(container, holder, &budget)
 	for _, c := range holder.Children {
 		c.Parent = nil
 	}
diff --git a/dom/mutate_test.go b/dom/mutate_test.go
index 0bbe0d3..d5be13f 100644
--- a/dom/mutate_test.go
+++ b/dom/mutate_test.go
@@ -3,7 +3,10 @@
 
 package dom
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestNewElementAndText(t *testing.T) {
 	el := NewElement("DIV")
@@ -248,6 +251,37 @@ func TestParseFragment(t *testing.T) {
 	}
 }
 
+func countTreeNodes(nodes []*Node) int {
+	c := 0
+	for _, n := range nodes {
+		c++
+		c += countTreeNodes(n.Children)
+		if n.Content != nil {
+			c++
+			c += countTreeNodes(n.Content.Children)
+		}
+	}
+	return c
+}
+
+func TestParseFragmentCapsTotalNodeCount(t *testing.T) {
+	// A WIDE document (many sibling elements, never nesting past a handful
+	// of levels) sails straight through golang.org/x/net/html's own
+	// 512-level DEPTH guard untouched — that guard is about depth, not
+	// total node count. Confirmed live: a single statement,
+	// `root.innerHTML = "<div>x</div>".repeat(3000000)` (SetInnerHTML calls
+	// ParseFragment), allocated ~11.5GB of real memory in 15 seconds with
+	// no loop and no deep nesting at all — the 13-byte repeated fragment is
+	// nowhere near a pathologically large string by itself.
+	nodes, err := ParseFragment(strings.Repeat("<div>x</div>", maxParsedNodes+1000))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := countTreeNodes(nodes); got > maxParsedNodes {
+		t.Errorf("ParseFragment produced %d nodes, want <= %d (maxParsedNodes)", got, maxParsedNodes)
+	}
+}
+
 func TestSerializeTextNode(t *testing.T) {
 	// A bare text node serializes (and escapes) directly.
 	if got := OuterHTML(NewText("<b>")); got != "&lt;b&gt;" {