Repository navigation
security: cap canvas pixel count and cloneNode's total node output - #276
Merged
Merged
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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.Interrupt()only takes effect at goja's next bytecode instruction; a single already-in-flightcloneNode(true)call is one uninterruptible native Go call, and that one call alone balloons memory once its input tree is already large (confirmed: settingMaxJSHeapBytesto 100MB produced identical total memory to the 768MB default).Fixed two ways:
cloneNodenow carries a node-count budget (maxCloneNodes=200,000) shared across one call's whole recursion, converting exponential-per-call growth into linear.newCanvasgains a total-pixel-count cap (maxCanvasPixels=200,000,000, 7× 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).
Test plan
go build ./...go test ./...(all packages pass)TestCloneNodeCapsTotalNodeCountPerCall,TestCloneNodeCapsTotalNodeCountInsideTemplateContent,TestNewCanvasCapsTotalPixelCount,TestNewCanvasOrdinaryHeightUnaffected,TestNewCanvasClampFloorHoldsWhenViewportAloneExceedsCapnewCanvasandcloneNodeDepthboth at 100% coverage🤖 Generated with Claude Code