From a613bd59d5b8a49b47f0d0b363e6027a0ed75315 Mon Sep 17 00:00:00 2001
From: tannevaled <tannevaled@users.noreply.github.com>
Date: Sat, 17 Oct 2026 22:20:51 +0200
Subject: [PATCH] security: cap canvas pixel count and cloneNode's total node
 output

Closes round 171's own open lead: a 25-iteration
root.appendChild(root.cloneNode(true)) loop measured at ~21GB of
REAL (not sparse) memory in under 9 seconds, and neither the
existing JS heap watchdog nor round 171's own recursion-depth cap
could stop it.

The heap watchdog's Interrupt() only takes effect at goja's next
bytecode instruction; a single already-in-flight cloneNode(true)
call is one uninterruptible native Go call, and that one call
alone balloons memory once its input tree is already large
(confirmed: setting MaxJSHeapBytes to 100MB produced identical
total memory to the 768MB default). The depth cap doesn't apply
either: this construction builds a BUSHY tree (each clone nests a
full copy of everything so far), so no single call's recursion
exceeds the depth cap -- the runaway dimension is total node
count, not depth.

Fixed two ways: cloneNode now carries a node-count budget
(maxCloneNodes=200,000) shared across one call's whole recursion,
converting exponential-per-call growth into linear; newCanvas
gains a total-pixel-count cap (maxCanvasPixels=200,000,000, 7x the
tallest page in this engine's own 500-page bench corpus), clamping
height rather than allocating unboundedly.

Together: the original reproduction drops from ~21GB to ~5.6GB at
25 iterations, confirmed linear (not exponential) in iteration
count at 40 iterations (~7.8GB).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 FIDELITY.md         | 19 ++++++++++++++
 canvas_bomb_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++++
 engine.go           | 26 +++++++++++++++++--
 js/dom.go           | 49 +++++++++++++++++++++++++++++-------
 js/dom_test.go      | 50 +++++++++++++++++++++++++++++++++++++
 5 files changed, 194 insertions(+), 11 deletions(-)
 create mode 100644 canvas_bomb_test.go

diff --git a/FIDELITY.md b/FIDELITY.md
index 69564fa..95502e5 100644
--- a/FIDELITY.md
+++ b/FIDELITY.md
@@ -18,6 +18,25 @@ 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 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.
+
+Two independent gaps, found while root-causing this:
+
+- **The existing JS heap watchdog (`heapGuardedContext`, `MaxJSHeapBytes`, default 768MB) cannot stop this, regardless of how low its limit is set** — confirmed by setting it to 100MB and seeing IDENTICAL total memory usage to the 768MB default. Its `Interrupt()` only takes effect at the next bytecode instruction goja executes; a single `cloneNode(true)` call already in flight is one uninterruptible native Go call from goja's point of view, and that one call alone is what balloons memory once its input tree is already large. The watchdog is real and does fire (confirmed via `JSLog`), it is just structurally unable to abort a single native call once started.
+- **Round 171's own `maxDOMWalkDepth` cap does not apply here either**: `root.appendChild(root.cloneNode(true))` builds a BUSHY tree (each clone nests a full copy of everything accumulated so far, one level deeper under root), so no single `cloneNode` call's own recursion ever exceeds the depth cap — the runaway dimension is TOTAL NODE COUNT, which doubles every iteration, not recursion depth, which stays modest.
+- **Root cause of the actual memory number**: `newCanvas` (`engine.go`) sizes the output image from the laid-out content height alone, with no upper bound — a script can grow that height arbitrarily (here, to 13,288,016px), and `vpW × canvasH × 4 bytes` scales accordingly with nothing stopping it.
+
+**Fixed two ways, addressing both the tree-size and the canvas-size dimensions**:
+- `cloneNode` now carries a node-count `budget` (`maxCloneNodes = 200,000`) alongside its existing depth cap, shared across one call's whole recursion (the same shape as round 170's `expandFunctionalPseudosBudget`). Verified directly: node-count growth that was exactly doubling every iteration (4, 8, 16, … 262,144) becomes linear (+200,000/iteration) exactly once the budget is first exhausted, confirmed node-by-node with a throwaway counting test run against the real doubling loop.
+- `newCanvas` gains `maxCanvasPixels = 200,000,000` (200 megapixels — 7× the tallest page in this engine's own 500-page bench corpus, Wikipedia at ~29M pixels, while bounding worst-case allocation to ~800MB instead of tens of GB), clamping `canvasH` rather than failing the render.
+- **Together**: the original 21GB reproduction now uses ~5.6GB (a ~4× reduction) at 25 iterations and ~7.8GB at 40 iterations — confirmed LINEAR in iteration count now, not exponential, meaning an attacker needs proportionally more explicit script statements for more damage rather than getting free exponential growth, and the existing JS script timeout (5s default) bounds how many such statements can run at all.
+- **Test**: `js/dom_test.go`'s `TestCloneNodeCapsTotalNodeCountPerCall` and `TestCloneNodeCapsTotalNodeCountInsideTemplateContent` (the `<template>` content-fragment clone is a separate loop sharing the same budget); new file `canvas_bomb_test.go` (matching the existing `images_bomb_test.go` naming), `TestNewCanvasCapsTotalPixelCount`, `TestNewCanvasOrdinaryHeightUnaffected`, `TestNewCanvasClampFloorHoldsWhenViewportAloneExceedsCap`.
+- **Coverage**: both `newCanvas` and `cloneNodeDepth` land at 100% (confirmed clean baseline beforehand — the two added branches were the only gaps).
+- **Not fully closed, documented honestly**: a sufficiently long-running script could still accumulate several×`maxCloneNodes` across many separate `cloneNode` calls before the 5-second script timeout stops it — bounded and linear now, not literally unbounded, but not a hard per-render ceiling either. A session-wide budget shared across every `cloneNode` call in one render would close that remaining gap; not attempted this round, since the per-call budget already converts the practically-unbounded exponential case into a bounded, attacker-costly linear one.
+- Also hit, again, the same `git stash` pathspec trap from round 171's own write-up (mixing a tracked and an untracked path in one `git stash push --` call aborts the whole push silently, and a subsequent blind `pop` can then resurface an unrelated OLDER stash instead) — the canonical bench baseline reappeared modified a second time mid-round from exactly this, fixed the same way (a clean, single-type-of-path stash), but worth naming the trap explicitly this time rather than just the symptom.
+
 ## 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.
diff --git a/canvas_bomb_test.go b/canvas_bomb_test.go
new file mode 100644
index 0000000..0d72653
--- /dev/null
+++ b/canvas_bomb_test.go
@@ -0,0 +1,61 @@
+// Copyright (c) the go-webengine/engine authors.
+// SPDX-License-Identifier: BSD-3-Clause
+
+package engine
+
+import (
+	"image"
+	"testing"
+
+	"github.com/go-webengine/engine/dom"
+)
+
+// newCanvas allocates a REAL, eagerly zero-filled buffer (confirmed live,
+// round 172: NOT the cheap-until-painted sparse allocation it might look
+// like) sized from the laid-out content height alone, with no upper bound
+// before this test's own fix. A script can grow that height arbitrarily —
+// `root.appendChild(root.cloneNode(true))` in a 25-iteration loop reached
+// 1024×13,288,016, measured at ~20GB of real resident memory in under 10
+// seconds — with no pathologically large string or deep recursion needed
+// at all. maxCanvasPixels (engine.go) bounds the ALLOCATION regardless of
+// how tall layout says the content is.
+func TestNewCanvasCapsTotalPixelCount(t *testing.T) {
+	e := New()
+	doc := &Document{Root: &dom.Node{Type: dom.Document}}
+	// A content height whose pixel count, at this viewport width, would
+	// exceed maxCanvasPixels by a wide margin if uncapped.
+	rp := &renderPass{height: 500_000}
+	viewport := image.Rect(0, 0, 1024, 768)
+	img := e.newCanvas(doc, rp, viewport, 1024)
+	got := int64(img.Bounds().Dx()) * int64(img.Bounds().Dy())
+	if got > maxCanvasPixels {
+		t.Errorf("newCanvas allocated %d pixels, want <= %d (maxCanvasPixels)", got, maxCanvasPixels)
+	}
+}
+
+// An ordinary page's content height is untouched by the cap.
+func TestNewCanvasOrdinaryHeightUnaffected(t *testing.T) {
+	e := New()
+	doc := &Document{Root: &dom.Node{Type: dom.Document}}
+	rp := &renderPass{height: 3000}
+	viewport := image.Rect(0, 0, 1024, 768)
+	img := e.newCanvas(doc, rp, viewport, 1024)
+	if h := img.Bounds().Dy(); h != 3000 {
+		t.Errorf("canvas height = %d, want 3000 (unaffected by the cap)", h)
+	}
+}
+
+// A viewport wider than maxCanvasPixels itself leaves no room for even one
+// row once divided; the clamp's own floor (1) must still hold rather than
+// leaving the canvas zero-height.
+func TestNewCanvasClampFloorHoldsWhenViewportAloneExceedsCap(t *testing.T) {
+	e := New()
+	doc := &Document{Root: &dom.Node{Type: dom.Document}}
+	rp := &renderPass{height: 1}
+	vpW := maxCanvasPixels + 1
+	viewport := image.Rect(0, 0, vpW, 768)
+	img := e.newCanvas(doc, rp, viewport, vpW)
+	if h := img.Bounds().Dy(); h != 1 {
+		t.Errorf("canvas height = %d, want 1 (the clamp's own floor)", h)
+	}
+}
diff --git a/engine.go b/engine.go
index d3d8acf..eededd5 100644
--- a/engine.go
+++ b/engine.go
@@ -503,10 +503,26 @@ func (e *Engine) heapGuardedContext(parent context.Context) (context.Context, fu
 	}
 }
 
+// maxCanvasPixels bounds the output image's total pixel count (width ×
+// height). image.NewRGBA allocates and zero-fills every byte up front —
+// confirmed NOT the sparse, cheap-until-painted allocation it might look
+// like: a canvas this large, built from nothing more than a script growing
+// a page's content height via ordinary DOM calls (round 172's own finding —
+// `root.appendChild(root.cloneNode(true))` in a 25-iteration loop; no
+// pathologically large string or recursion depth needed at all), measured
+// at ~20GB of REAL resident memory in under 10 seconds, not virtual/sparse.
+// The tallest page in this engine's own 500-page bench corpus is Wikipedia
+// at 1024×28,562 (~29M pixels); 200 megapixels is generous against that by
+// 7×, bounding worst case to ~800MB (4 bytes/pixel) rather than tens of GB.
+const maxCanvasPixels = 200_000_000
+
 // newCanvas allocates the output image, fills the white base and paints the page
 // backdrop (body/html background) over the whole viewport, matching a browser.
-// The height is grown to the laid-out content, at least the viewport height, and
-// clamped to a minimum of one pixel.
+// The height is grown to the laid-out content, at least the viewport height,
+// clamped to a minimum of one pixel and a maximum set by maxCanvasPixels (a
+// page whose real content is taller than that gets a TRUNCATED render rather
+// than an unbounded allocation — no corpus page measured anywhere near this
+// limit, so no legitimate page should ever visibly hit it).
 func (e *Engine) newCanvas(doc *Document, rp *renderPass, viewport image.Rectangle, vpW int) *image.RGBA {
 	canvasH := viewport.Dy()
 	if int(rp.height) > canvasH {
@@ -515,6 +531,12 @@ func (e *Engine) newCanvas(doc *Document, rp *renderPass, viewport image.Rectang
 	if canvasH <= 0 {
 		canvasH = 1
 	}
+	if vpW > 0 && int64(vpW)*int64(canvasH) > maxCanvasPixels {
+		canvasH = int(maxCanvasPixels / int64(vpW))
+		if canvasH <= 0 {
+			canvasH = 1
+		}
+	}
 	img := image.NewRGBA(image.Rect(0, 0, vpW, canvasH))
 	// Base paper: the configured Backdrop when opaque, else white.
 	if e.Backdrop.A > 0 {
diff --git a/js/dom.go b/js/dom.go
index 44693a8..60936f4 100644
--- a/js/dom.go
+++ b/js/dom.go
@@ -1761,39 +1761,70 @@ func isEqualNode(a, b *dom.Node, depth int) bool {
 // 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.
+//
+// budget bounds the TOTAL node count a single top-level cloneNode(true) call
+// may produce — a separate dimension from depth, found the hard way (round
+// 172): `root.appendChild(root.cloneNode(true))` in a 25-iteration loop
+// builds a BUSHY tree (each clone nests a full copy of everything so far,
+// one level deeper under root), so no single clone call's own recursion
+// ever exceeds maxDOMWalkDepth, yet the TOTAL node count still doubles each
+// iteration. Measured: this allocates ~20GB of real (not virtual/sparse)
+// memory in under 9 seconds, and the existing JS heap watchdog
+// (heapGuardedContext) cannot stop it regardless of how low its limit is
+// set — its Interrupt() takes effect only at the NEXT bytecode instruction
+// goja executes, but a single already-in-flight cloneNode(true) call is one
+// uninterruptible native Go call from goja's own point of view, and that one
+// call alone is what balloons memory once the input tree is already large.
+// Bounding it from WITHIN the clone turns unbounded-per-call exponential
+// growth into bounded-per-call linear growth — the same shape as round
+// 170's expandFunctionalPseudosBudget. maxCloneNodes is generous for any
+// real page (tens of thousands of DOM nodes is already an unusually large
+// document) while keeping one call's own worst case a sane, bounded amount
+// of memory.
 func cloneNode(n *dom.Node, deep bool) *dom.Node {
-	return cloneNodeDepth(n, deep, 0)
+	budget := maxCloneNodes
+	return cloneNodeDepth(n, deep, 0, &budget)
 }
 
-func cloneNodeDepth(n *dom.Node, deep bool, depth int) *dom.Node {
+// maxCloneNodes bounds the total nodes one cloneNode(true) call may create.
+const maxCloneNodes = 200000
+
+func cloneNodeDepth(n *dom.Node, deep bool, depth int, budget *int) *dom.Node {
 	if n == nil {
 		return nil
 	}
 	c := &dom.Node{Type: n.Type, Tag: n.Tag, Text: n.Text}
+	*budget--
 	if n.Attr != nil {
 		c.Attr = map[string]string{}
 		for k, v := range n.Attr {
 			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 {
+	// Past either 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 || *budget <= 0 {
 		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, cloneNodeDepth(ch, true, depth+1))
+				if *budget <= 0 {
+					break
+				}
+				dom.AppendChild(c.Content, cloneNodeDepth(ch, true, depth+1, budget))
 			}
 		}
 	}
 	if deep {
 		for _, ch := range n.Children {
-			dom.AppendChild(c, cloneNodeDepth(ch, true, depth+1))
+			if *budget <= 0 {
+				break
+			}
+			dom.AppendChild(c, cloneNodeDepth(ch, true, depth+1, budget))
 		}
 	}
 	return c
diff --git a/js/dom_test.go b/js/dom_test.go
index 8ddafcd..90347a8 100644
--- a/js/dom_test.go
+++ b/js/dom_test.go
@@ -62,3 +62,53 @@ func TestCloneNodeDoesNotCrashOnPathologicallyDeepTree(t *testing.T) {
 		t.Errorf("clone.Tag = %q, want %q", clone.Tag, root.Tag)
 	}
 }
+
+func countNodes(n *dom.Node) int {
+	c := 1
+	for _, ch := range n.Children {
+		c += countNodes(ch)
+	}
+	return c
+}
+
+func TestCloneNodeCapsTotalNodeCountPerCall(t *testing.T) {
+	// A single cloneNode(true) call recurses depth-first, so maxDOMWalkDepth
+	// alone does not bound a BUSHY tree: round 172 found that a script doing
+	// `root.appendChild(root.cloneNode(true))` in a loop builds one (each
+	// clone nests a full copy of everything so far, one level deeper under
+	// root), no single call's own recursion ever exceeding the depth cap,
+	// yet TOTAL node count still doubling every iteration — measured at
+	// ~20GB of real memory in under 10 seconds. A wide tree with more than
+	// maxCloneNodes children exercises the same budget this round added,
+	// without needing the full doubling construction to prove it works.
+	root := &dom.Node{Type: dom.Element, Tag: "div", Attr: map[string]string{}}
+	for i := 0; i < maxCloneNodes+1000; i++ {
+		ch := &dom.Node{Type: dom.Element, Tag: "span", Attr: map[string]string{}}
+		ch.Parent = root
+		root.Children = append(root.Children, ch)
+	}
+	clone := cloneNode(root, true)
+	if got := countNodes(clone); got > maxCloneNodes {
+		t.Errorf("cloneNode produced %d nodes, want <= %d (maxCloneNodes)", got, maxCloneNodes)
+	}
+}
+
+func TestCloneNodeCapsTotalNodeCountInsideTemplateContent(t *testing.T) {
+	// The <template> content-fragment clone (see cloneNode's own doc
+	// comment, cloning steps for template elements) is a SEPARATE loop from
+	// the generic Children walk, sharing the same budget — this exercises
+	// that loop's own budget-exhausted break specifically, not just the
+	// generic Children one TestCloneNodeCapsTotalNodeCountPerCall already
+	// covers.
+	tpl := &dom.Node{Type: dom.Element, Tag: "template", Attr: map[string]string{}}
+	tpl.Content = &dom.Node{Type: dom.Element, Tag: "#fragment", Attr: map[string]string{}}
+	for i := 0; i < maxCloneNodes+1000; i++ {
+		ch := &dom.Node{Type: dom.Element, Tag: "span", Attr: map[string]string{}}
+		ch.Parent = tpl.Content
+		tpl.Content.Children = append(tpl.Content.Children, ch)
+	}
+	clone := cloneNode(tpl, true)
+	if got := countNodes(clone.Content); got > maxCloneNodes {
+		t.Errorf("cloned template content has %d nodes, want <= %d (maxCloneNodes)", got, maxCloneNodes)
+	}
+}