[pkg/pdatautil] Hashing Maps can panic with nested maps

[cpheps]

Component(s)

pkg/pdatautil

What happened?

Description

writeMapHash can panic when iterating through nested maps as hw.keysBuf is overwritten every iteration. If a nested map key is not alphabetically last of all the keys in the map and it has at least one more key than its index in the parent map it'll cause the next iteration of the parent keys to try and find a child key instead.

Steps to Reproduce

I created this test in hash_test.go that replicates the issue

func TestMapHashPanic(t *testing.T) {
	m := pcommon.NewMap()
	m.PutInt("a", 1)
	m.PutInt("c", 1)
	m.PutInt("d", 1)
	m.PutInt("e", 1)
       
        // nestedMap is alphabetically second in parent map
	nestedMap := m.PutEmptyMap("b")

        // nestedMap is index 1 in parent when keys are sorted so we need at least 3 keys in this to ensure the next iteration of parent tries to find a key at "a3"
	nestedMap.PutInt("a1", 1)
	nestedMap.PutInt("a2", 1)
	nestedMap.PutInt("a3", 1)

	assert.NotPanics(t, func() { MapHash(m) })
}

Expected Result

No Panic

Actual Result

Panic due to invalid key access

Panic stack from test:

func (assert.PanicTestFunc)(0x10514e600) should not panic
Panic value:	runtime error: invalid memory address or nil pointer dereference
Panic stack:	goroutine 34 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x64
github.com/stretchr/testify/assert.didPanic.func1()
	/Users/cphelps/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:1050 +0x74
panic({0x1052ab4e0, 0x10570c4a0})
	/usr/local/go/src/runtime/panic.go:884 +0x1f4
go.opentelemetry.io/collector/pdata/pcommon.Value.Type(...)
	/Users/cphelps/go/pkg/mod/go.opentelemetry.io/collector/[email protected]/pcommon/value.go:178
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil.(*hashWriter).writeValueHash(0x140001285a0?, {0x1400013c380?})
	/Users/cphelps/git/opentelemetry-collector-contrib/pkg/pdatautil/hash.go:109 +0x54
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil.(*hashWriter).writeMapHash(0x140001edf80, {0x100019798?})
	/Users/cphelps/git/opentelemetry-collector-contrib/pkg/pdatautil/hash.go:98 +0xe8
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil.MapHash({0x1052a7f80?})
	/Users/cphelps/git/opentelemetry-collector-contrib/pkg/pdatautil/hash.go:72 +0xd0
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil.TestMapHashPanic.func1()
	/Users/cphelps/git/opentelemetry-collector-contrib/pkg/pdatautil/hash_test.go:150 +0x20
github.com/stretchr/testify/assert.didPanic(0x140001a0600?)
	/Users/cphelps/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:1055 +0x7c
github.com/stretchr/testify/assert.NotPanics({0x105357468, 0x1400011f520}, 0x1400010b010, {0x0, 0x0, 0x0})
	/Users/cphelps/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:1126 +0x64
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil.TestMapHashPanic(0x0?)
	/Users/cphelps/git/opentelemetry-collector-contrib/pkg/pdatautil/hash_test.go:150 +0x144
testing.tRunner(0x1400011f520, 0x105352998)
	/usr/local/go/src/testing/testing.go:1576 +0x104
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1629 +0x370

Collector version

v0.72.0

Environment information

Environment

OS: macOS 13.2.1
Compiler(if manually compiled): go 1.20.1

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

I think the keyset that's iterated through likely needs to be stored in a variable that's local to the writeMapHash function. Every nested map will overwrite that keyset for it's parent.

[github-actions]

Pinging code owners:

• pkg/pdatautil: @dmitryax

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[dmitryax]

@cpheps thanks for fixing it!