mirror of
https://github.com/mikefarah/yq.git
synced 2026-07-01 18:01:40 +00:00
Fix stack overflow from circular alias in traverse (#2647)
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>
This commit is contained in:
parent
2ef934281e
commit
0374ad6b4b
@ -36,9 +36,33 @@ func traversePathOperator(_ *dataTreeNavigator, context Context, expressionNode
|
||||
return context.ChildContext(matches), nil
|
||||
}
|
||||
|
||||
// resolveAliasChain follows an alias chain iteratively, returning the
|
||||
// first non-alias node. Returns an error if a cycle is detected.
|
||||
func resolveAliasChain(node *CandidateNode) (*CandidateNode, error) {
|
||||
if node.Kind != AliasNode {
|
||||
return node, nil
|
||||
}
|
||||
visited := map[*CandidateNode]bool{}
|
||||
for node.Kind == AliasNode {
|
||||
if visited[node] {
|
||||
return nil, fmt.Errorf("alias cycle detected")
|
||||
}
|
||||
visited[node] = true
|
||||
log.Debug("its an alias!")
|
||||
node = node.Alias
|
||||
}
|
||||
return node, nil
|
||||
}
|
||||
|
||||
func traverse(context Context, matchingNode *CandidateNode, operation *Operation) (*list.List, error) {
|
||||
log.Debugf("Traversing %v", NodeToString(matchingNode))
|
||||
|
||||
var err error
|
||||
matchingNode, err = resolveAliasChain(matchingNode)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if matchingNode.Tag == "!!null" && operation.Value != "[]" && !context.DontAutoCreate {
|
||||
log.Debugf("Guessing kind")
|
||||
// we must have added this automatically, lets guess what it should be now
|
||||
@ -62,10 +86,6 @@ func traverse(context Context, matchingNode *CandidateNode, operation *Operation
|
||||
log.Debugf("its a sequence of %v things!", len(matchingNode.Content))
|
||||
return traverseArray(matchingNode, operation, operation.Preferences.(traversePreferences))
|
||||
|
||||
case AliasNode:
|
||||
log.Debug("its an alias!")
|
||||
matchingNode = matchingNode.Alias
|
||||
return traverse(context, matchingNode, operation)
|
||||
default:
|
||||
return list.New(), nil
|
||||
}
|
||||
@ -125,7 +145,13 @@ func traverseNodesWithArrayIndices(context Context, indicesToTraverse []*Candida
|
||||
return context.ChildContext(matchingNodeMap), nil
|
||||
}
|
||||
|
||||
func traverseArrayIndices(context Context, matchingNode *CandidateNode, indicesToTraverse []*CandidateNode, prefs traversePreferences) (*list.List, error) { // call this if doc / alias like the other traverse
|
||||
func traverseArrayIndices(context Context, matchingNode *CandidateNode, indicesToTraverse []*CandidateNode, prefs traversePreferences) (*list.List, error) {
|
||||
var err error
|
||||
matchingNode, err = resolveAliasChain(matchingNode)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if matchingNode.Tag == "!!null" {
|
||||
log.Debugf("OperatorArrayTraverse got a null - turning it into an empty array")
|
||||
// auto vivification
|
||||
@ -138,9 +164,6 @@ func traverseArrayIndices(context Context, matchingNode *CandidateNode, indicesT
|
||||
}
|
||||
|
||||
switch matchingNode.Kind {
|
||||
case AliasNode:
|
||||
matchingNode = matchingNode.Alias
|
||||
return traverseArrayIndices(context, matchingNode, indicesToTraverse, prefs)
|
||||
case SequenceNode:
|
||||
return traverseArrayWithIndices(matchingNode, indicesToTraverse, prefs)
|
||||
case MappingNode:
|
||||
|
||||
@ -665,6 +665,16 @@ var traversePathOperatorScenarios = []expressionScenario{
|
||||
"D0, P[a], (!!null)::null\n",
|
||||
},
|
||||
},
|
||||
{
|
||||
// Regression test for https://issues.oss-fuzz.com/issues/390467412
|
||||
// go-yaml accepts cross-document alias references (invalid per
|
||||
// YAML spec). A nested assignment on such an alias can create a
|
||||
// circular alias node, which must not cause a stack overflow.
|
||||
skipDoc: true,
|
||||
document: "&-- a\n---\n*--",
|
||||
expression: ". = (.x = 1)",
|
||||
expectedError: "alias cycle detected",
|
||||
},
|
||||
}
|
||||
|
||||
func TestTraversePathOperatorScenarios(t *testing.T) {
|
||||
@ -682,3 +692,58 @@ func TestTraversePathOperatorAlignedToSpecScenarios(t *testing.T) {
|
||||
appendOperatorDocumentScenario(t, "traverse-read", fixedTraversePathOperatorScenarios)
|
||||
ConfiguredYamlPreferences.FixMergeAnchorToSpec = false
|
||||
}
|
||||
|
||||
// Regression test for https://issues.oss-fuzz.com/issues/390467412
|
||||
// A circular alias (alias pointing back to itself) must not cause a
|
||||
// stack overflow. resolveAliasChain should detect the cycle and return
|
||||
// an error; both traverse() and traverseArrayIndices() use it.
|
||||
func TestTraverseAliasCycle(t *testing.T) {
|
||||
aliasNode := &CandidateNode{
|
||||
Kind: AliasNode,
|
||||
}
|
||||
aliasNode.Alias = aliasNode // A -> A
|
||||
|
||||
op := &Operation{
|
||||
OperationType: traversePathOpType,
|
||||
Value: "key",
|
||||
StringValue: "key",
|
||||
Preferences: traversePreferences{},
|
||||
}
|
||||
_, err := traverse(Context{}, aliasNode, op)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for alias cycle, got nil")
|
||||
}
|
||||
if err.Error() != "alias cycle detected" {
|
||||
t.Fatalf("expected 'alias cycle detected', got %q", err.Error())
|
||||
}
|
||||
|
||||
// Same cycle must be caught through the array traversal path.
|
||||
_, err = traverseArrayIndices(Context{}, aliasNode, nil, traversePreferences{})
|
||||
if err == nil {
|
||||
t.Fatal("expected error for alias cycle via traverseArrayIndices, got nil")
|
||||
}
|
||||
if err.Error() != "alias cycle detected" {
|
||||
t.Fatalf("expected 'alias cycle detected', got %q", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestTraverseAliasCycleChain(t *testing.T) {
|
||||
nodeA := &CandidateNode{Kind: AliasNode}
|
||||
nodeB := &CandidateNode{Kind: AliasNode}
|
||||
nodeA.Alias = nodeB
|
||||
nodeB.Alias = nodeA // A -> B -> A
|
||||
|
||||
op := &Operation{
|
||||
OperationType: traversePathOpType,
|
||||
Value: "key",
|
||||
StringValue: "key",
|
||||
Preferences: traversePreferences{},
|
||||
}
|
||||
_, err := traverse(Context{}, nodeA, op)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for alias cycle chain, got nil")
|
||||
}
|
||||
if err.Error() != "alias cycle detected" {
|
||||
t.Fatalf("expected 'alias cycle detected', got %q", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user