pkg/profile: bounds-check DecodeInto

[guettlibot]

Independent of #6415, #6416, #6417 and #6418 — branches from main, touches only pkg/profile/decode.go.

This is the one on the default path. The other PRs in this batch are confined to the ClickHouse backend; DecodeInto is what pkg/parcacol/querier.go:1133 uses to read locations back, so it runs for every query against the FrostDB backend.

What

DecodeInto indexes and slices caller-supplied bytes with no length checks at all. Feeding it every prefix of one real encoded location:

MEASURED DecodeInto: panics=55 of 66 prefixes

And there is no safety net:

$ grep -rn 'recover()' pkg cmd
pkg/symbol/addr2line/dwarf.go: ...
pkg/symbol/addr2line/go.go:    ...
(nothing else; no gRPC recovery interceptor either)

So a corrupt or truncated stored location is a dead server, not a failed query. These bytes have been to storage and back, which makes corruption a more plausible route to a malformed record than an in-process round trip.

Three ways it could fault

• data[offset] for the two flag bytes, past the end of a short record.
• data[offset:] for every varint, likewise.
• decodeString's data[n : n+int(length)]. The length is unsigned: one above MaxInt converts to a negative int, so n+int(length) lands below n. That satisfies a naive n+int(length) > len(data) guard and then panics on a slice whose high bound is below its low one — so the check has to be made in the space the length was read in.

Panicking is not the worst outcome

These bytes usually arrive as an Arrow value, and array.Binary.Value returns a two-index slice whose cap runs to the end of the whole buffer. A read past len but inside cap therefore does not fault — it returns the bytes of the next location, which then decode as this location's own mapping or function. Plausible-looking, and nothing would flag it.

Fix

Each read reports whether it succeeded, and a record that runs out returns a descriptive error naming the field and offset:

malformed location: ran out of bytes reading mapping build ID at offset 12 of 11

The signature already returns an error, so there is no reason to guess: a malformed location fails the query that touched it rather than degrading quietly or killing the process.

decodeString itself is left alone — it has thirteen other call sites across pkg/profile and pkg/symbolizer, several belonging to decoders with their own self-consistent formats, so changing its contract is a wider change than this one. The checks live in DecodeInto, matching the shape already used by DecodeSymbolizationInfo.

Tests

Behaviour on well-formed input is unchanged (the existing TestEncodeDecode and TestDecodeFallsBackToSystemName pin that). The truncation sweep runs over five record shapes, because each reaches a different set of reads, and slices each prefix as encoded[:i:i] — a two-index slice drops the original cap, and Go does not panic reading past len while still inside cap, so the cheaper spelling silently passes on inputs that really do over-read. Oversized lengths (1<<20, 1<<63, MaxUint64) are covered too.

Verified the new tests fail on main (should not panic). go test (incl. -race), go vet, gofmt, license check clean across pkg/profile, pkg/parcacol, pkg/normalizer, pkg/clickhouse, pkg/parca, pkg/symbolizer.

Not in this PR

pkg/symbolizer has its own decodeString and decodeLines with the same unchecked style. That pair is self-consistent with pkg/symbolizer/encode.go and has a round-trip test, so it is a separate, lower-risk case — happy to follow up if you'd like it hardened too.