Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 31 additions & 8 deletions pkg/yqlib/operator_traverse_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
65 changes: 65 additions & 0 deletions pkg/yqlib/operator_traverse_path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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