pkg/clickhouse, pkg/profile: bounds-check the location decoders

[guettlibot]

Stacked on #6415 — review that one first; this branch contains its commit as a parent. The diff specific to this PR is the second commit.

What

The ClickHouse ingest path decodes every encoded location twice — profile.DecodeSymbolizationInfo for the address and mapping, then clickhouse.decodeLineInfo for the line and function — and neither had a single length check.

Feeding every prefix of one real encoded location to them:

decoder	truncations that panic	after
profile.DecodeSymbolizationInfo	19 of 66	0
clickhouse.decodeLineInfo	55 of 66	0

grep -rn 'recover()' pkg cmd finds nothing on this path, so a malformed record is a dead server rather than a failed request.

Three ways they could fault

• data[offset] for the flag bytes, past the end of a short record.
• data[offset:] for every varint, likewise.
• data[offset : offset+int(length)] for the strings. The length is unsigned: one above MaxInt converts to a negative int, so offset+int(length) lands below offset. That satisfies a naive offset+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 was not the worst outcome

In production these bytes come from an Arrow dictionary buffer, and array.Binary.Value returns a two-index slice whose cap runs to the end of the whole buffer. A read past the end of one entry therefore does not fault — it silently returns the bytes of the next location, which then get stored as this location's function filename. Plausible-looking, in a column nothing would flag. Reproduced with a real array.BinaryBuilder: of 71 truncations of entry 0, 20 silently copied neighbouring bytes into FunctionFilename.

A failed read now yields "" — never a prefix, never a neighbour.

Partial, but not silent

A record that runs out yields whatever was decoded before it did: that is the honest answer for a truncated record, and it keeps one bad location from discarding the rest of the batch. But silence here would be its own bug — a systematic drift would degrade every profile to unnamed frames, indistinguishable from legitimately unsymbolized ones, which is the same silent failure #6415 is about. So decodeLineInfo reports malformed records, extractStacktraceData counts them, and Ingest logs one warning per batch with the count. numLines == 0 and a line with no function are valid shapes and are not counted.

Verification

Behaviour on well-formed input is unchanged. A differential harness ran the old and new implementations over 20,864 generated valid records — hasMapping × numLines ∈ {0,1,2,3} × hasFunction × string lengths {0, 1, 127, 128, 300} × numeric widths up to MaxUint64, plus 20,000 seeded-random records — with zero divergences on any LineInfo field.

The truncation sweeps run over six record shapes, because each reaches a different set of reads: the unguarded decoder faulted on 13 of 14 prefixes of a no-function record and 34 of 43 of a no-mapping one, paths a single full-record sweep never visits. Oversized lengths (1<<20, 1<<63, MaxUint64) are covered at the function and mapping string positions.

The sweeps slice encoded[:i:i], not encoded[: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 — it reports 45 where the capped slice reports 55. Given the Arrow cap behaviour above, the capped number is the one that describes production.

go test (including -race), go vet, gofmt -s and the license check are clean; the new tests fail on the pre-change code.

Deliberately left out

Both pre-existing, both happy to file separately:

1. profile.decodeString and profile.DecodeInto are unguarded in the same style with the same unsigned-length hazard, reached from pkg/parcacol/querier.go:1172/:1566 and pkg/symbolizer. decodeString has 14 call sites across two packages, so guarding it is a wider change than this one.
2. EncodeArrowLocation writes the hasFunction flag unconditionally (pkg/profile/encode.go:362) while serializedArrowLocationSize budgets the function block only if lineFunctionName.IsValid(i) — so a line with an invalid function name undersizes the buffer and the encoder overruns it.