Fix stack overflow from circular alias in traverse#2647
Merged
mikefarah merged 1 commit intomikefarah:masterfrom Apr 6, 2026
Merged
Fix stack overflow from circular alias in traverse#2647mikefarah merged 1 commit intomikefarah:masterfrom
mikefarah merged 1 commit intomikefarah:masterfrom
Conversation
go-yaml accepts cross-document alias references, which the YAML spec
forbids (anchors are scoped to a single document). When a nested
assignment targets such an alias, UpdateFrom copies the Alias field
between nodes, creating a self-referencing AliasNode. Both traverse()
and traverseArrayIndices() then follow this cycle indefinitely.
Extract resolveAliasChain(), which follows aliases iteratively with a
visited set and returns an error on cycles. Both traverse() and
traverseArrayIndices() now call it, eliminating the recursive alias
handling in both code paths.
Note: traverseMergeAnchor() also dereferences aliases (lines 358 and
371) but with single-step assignment, not recursion. A self-referencing
alias there falls through the kind switch silently rather than
crashing. Using resolveAliasChain() in that function would produce a
clear error instead of silently dropping the node.
Reproducer (stack overflow before this fix, returns error after):
echo '&-- a
---
*--' | yq eval-all '. = (.x = 1)'
Found by OSS-Fuzz via the lima project's FuzzEvaluateExpression target.
https://issues.oss-fuzz.com/issues/390467412
Signed-off-by: Jan Dubois <jan@jandubois.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Hey @ingydotnet What are your thoughts on this statement? We should detect at go-yaml level if it's relevant |
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.
go-yaml accepts cross-document alias references, which the YAML spec forbids (anchors are scoped to a single document). When a nested assignment targets such an alias,
UpdateFromcopies theAliasfield between nodes, creating a self-referencingAliasNode. Bothtraverse()andtraverseArrayIndices()then follow this cycle indefinitely.Extract
resolveAliasChain(), which follows aliases iteratively with a visited set and returns an error on cycles. Bothtraverse()andtraverseArrayIndices()now call it, eliminating the recursive alias handling in both code paths.Note:
traverseMergeAnchor()also dereferences aliases (lines 358 and 371) but with single-step assignment, not recursion. A self-referencing alias there falls through the kind switch silently rather than crashing. UsingresolveAliasChain()in that function would produce a clear error instead of silently dropping the node.Reproducer (stack overflow before this fix, returns error after):
Found by OSS-Fuzz via the lima project's FuzzEvaluateExpression target. https://issues.oss-fuzz.com/issues/390467412