Fix findInArray misuse on MappingNodes in equality and contains (#2645)

recurseNodeObjectEqual and containsObject both used findInArray to
locate keys in a MappingNode's Content array. findInArray steps by 1,
so it matches against both keys (even indices) and values (odd indices).

In recurseNodeObjectEqual, when a null key in the LHS matched a null
value in the RHS at the last position, rhs.Content[indexInRHS+1]
accessed an out-of-bounds index, causing a panic.

In containsObject, a %2 guard prevented the panic but introduced false
negatives: when a null value appeared before the actual null key,
findInArray returned the value's odd index, the guard rejected it, and
the function reported the key as missing.

Both functions now use findKeyInMap, which steps by 2 and compares only
key positions. The %2 guard in containsObject is removed.

Reproducer for the panic (recurseNodeObjectEqual):

    echo '? [{~: ~}]
    : v1
    ? [{2: ~}]
    : v2' | yq '. += .'

Reproducer for the false negative (containsObject):

    printf '? 1\n: ~\n? ~\n: x\n' | yq 'contains({~: "x"})'

Found by OSS-Fuzz via the lima project's FuzzEvaluateExpression target.
https://issues.oss-fuzz.com/issues/383860504

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:
Jan Dubois
2026-04-06 18:30:44 +10:00
committed by GitHub
co-authored by Claude Opus 4.6
parent 80139ae1cc
commit b0ba9589d7
5 changed files with 43 additions and 3 deletions
+1 -1
View File
@@ -80,7 +80,7 @@ func recurseNodeObjectEqual(lhs *CandidateNode, rhs *CandidateNode) bool {
key := lhs.Content[index]
value := lhs.Content[index+1]
indexInRHS := findInArray(rhs, key)
indexInRHS := findKeyInMap(rhs, key)
if indexInRHS == -1 || !recursiveNodeEqual(value, rhs.Content[indexInRHS+1]) {
return false
+18
View File
@@ -300,6 +300,24 @@ func TestRecurseNodeObjectEqual(t *testing.T) {
test.AssertResult(t, true, recurseNodeObjectEqual(obj1, obj2))
test.AssertResult(t, false, recurseNodeObjectEqual(obj1, obj3))
test.AssertResult(t, false, recurseNodeObjectEqual(obj1, obj4))
// A null key must not match a null value in the other map.
// Regression test for https://issues.oss-fuzz.com/issues/383860504
nullKey := &CandidateNode{Kind: ScalarNode, Tag: "!!null"}
nullVal := &CandidateNode{Kind: ScalarNode, Tag: "!!null"}
intKey := createScalarNode(2, "2")
intKey.Tag = "!!int"
intVal := &CandidateNode{Kind: ScalarNode, Tag: "!!null"}
mapWithNullKey := &CandidateNode{
Kind: MappingNode,
Content: []*CandidateNode{nullKey, nullVal},
}
mapWithIntKey := &CandidateNode{
Kind: MappingNode,
Content: []*CandidateNode{intKey, intVal},
}
test.AssertResult(t, false, recurseNodeObjectEqual(mapWithNullKey, mapWithIntKey))
}
func TestParseInt(t *testing.T) {
+12
View File
@@ -527,6 +527,18 @@ var addOperatorScenarios = []expressionScenario{
expression: `.a += [2]`,
expectedError: "!!seq () cannot be added to a !!str (a)",
},
{
// Regression test for https://issues.oss-fuzz.com/issues/383860504
// Adding a map to itself must not panic when sequence keys contain
// single-entry mappings with a null key in one and a non-null key
// in the other.
skipDoc: true,
document: "? [{~: ~}]\n: v1\n? [{2: ~}]\n: v2",
expression: `. += .`,
expected: []string{
"D0, P[], (!!map)::? [{~: ~}]\n: v1\n? [{2: ~}]\n: v2\n",
},
},
}
func TestAddOperatorScenarios(t *testing.T) {
+2 -2
View File
@@ -46,9 +46,9 @@ func containsObject(lhs *CandidateNode, rhs *CandidateNode) (bool, error) {
rhsKey := rhs.Content[index]
rhsValue := rhs.Content[index+1]
log.Debugf("Looking for %v in the lhs", rhsKey.Value)
lhsKeyIndex := findInArray(lhs, rhsKey)
lhsKeyIndex := findKeyInMap(lhs, rhsKey)
log.Debugf("index is %v", lhsKeyIndex)
if lhsKeyIndex < 0 || lhsKeyIndex%2 != 0 {
if lhsKeyIndex < 0 {
return false, nil
}
lhsValue := lhs.Content[lhsKeyIndex+1]
+10
View File
@@ -65,6 +65,16 @@ var containsOperatorScenarios = []expressionScenario{
"D0, P[], (!!bool)::false\n",
},
},
{
// Regression: findInArray could match a null key against a null
// value at an earlier odd index, producing a false negative.
skipDoc: true,
document: "? 1\n: ~\n? ~\n: x",
expression: `contains({~: "x"})`,
expected: []string{
"D0, P[], (!!bool)::true\n",
},
},
{
description: "String contains substring",
document: `"foobar"`,