Repository navigation
security: cap DOM-walk recursion depth to stop a script-built tree from crashing the process - #265
Merged
Merged
Conversation
…om crashing the process golang.org/x/net/html already refuses to parse markup nested past 512 levels, but that guard lives in the parser alone: a page's own script can still build an arbitrarily deep tree at runtime (createElement + appendChild in a loop), since dom.AppendChild has no depth check of its own. Four unbounded recursive DOM walks (css.StylesheetLinks, styleElementText, css.Cascade's element walk, and layout's place/ contents) then hit an unrecoverable "fatal error: stack overflow" (not a panic -- recover() cannot catch it) between roughly 150,000 and 200,000 levels, killing the whole embedding process rather than just the one render. Each walk now caps its recursion at 512 (matching the parser's own bound) and stops descending past it instead of crashing. 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
golang.org/x/net/htmlalready refuses to parse markup nested past 512 levels, but that guard lives in the parser alone — a page's own<script>can build an arbitrarily deep tree at runtime (createElement+appendChildin a loop), sincedom.AppendChildhas no depth check of its own.css.StylesheetLinks,styleElementText,css.Cascade's element walk, andlayout'splace/contents) then hit an unrecoverablefatal error: stack overflow(not a panic —recover()cannot catch it) somewhere between 150,000 and 200,000 levels, confirmed with a synthetic tree built directly in Go. This kills the whole embedding process, not just the one render.@importcycles (maxImportDepth) andvar()cycles (maxVarDepth).layout.Boxtree, which layout's own cap keeps shallow.Test plan
go build ./...go test ./...(all packages pass)TestCascadeDoesNotCrashOnAPathologicallyDeepTree,TestStylesheetLinksDoesNotCrashOnAPathologicallyDeepTree(css),TestLayoutDoesNotCrashOnAPathologicallyDeepTree(layout) — each hand-builds a 10,000-level chain and confirms the walk returns instead of crashinglayoutpackage coverage restored to 100%;csspackage's pre-existing 99.5% floor is unaffected (confirmed identical before/after viagit stash)🤖 Generated with Claude Code