fix: heap overflows in V1/V2 log decode (crafted counts_limit / word_size)

[filipecosta90]

Security fix — heap buffer-overflow write, reachable from the public decode API

Surfaced by adversarial review during the new ClusterFuzzLite fuzzing effort (the random fuzzer didn't hit the specific crafted input in its budget; a reviewer found it by analysis).

Root cause

hdr_decode_compressed_v1 computes counts_limit = be32toh(payload_len) / word_size from the attacker-controlled payload_len field, then calls apply_to_counts(h, word_size, counts_array, counts_limit), which loops for (i=0; i<counts_limit; i++) h->counts[i] = .... With no bound against h->counts_len, a crafted log (tiny histogram + large payload_len) writes past the hdr_calloc(counts_len, 8) buffer → heap-buffer-overflow WRITE. Reachable via hdr_log_decode / hdr_log_read. V0 (passes h->counts_len) and V2 (bounds internally) are unaffected — only V1 trusted the payload length.

Fix

Reject counts_limit < 0 || counts_limit > h->counts_len with HDR_ENCODED_INPUT_TOO_LONG right after hdr_init, before allocation/apply. Also prevents an int32 overflow of counts_array_len = counts_limit * word_size.

Verification (local, ASan)

• Crafted a minimal malicious V1 blob. Before: AddressSanitizer: heap-buffer-overflow WRITE of size 8 in apply_to_counts_64 ← hdr_decode_compressed_v1:538 ← hdr_log_decode. After: returns HDR_ENCODED_INPUT_TOO_LONG, no ASan error.
• Added a regression test (test_v1_decode_rejects_oversized_counts) with the crafted blob; full suite passes under ASan+UBSan, valid encode/decode round-trips unaffected.

Note: the ASan/UBSan ctest CI job that turns this regression test into a deterministic guard is added in the companion PR #145; until that merges this PR's own CI runs the assertion in a normal build.

🤖 Generated with Claude Code

[github-actions]

🤖 Automated first-pass review — a human maintainer's review is still required before merge.

Read the diff against the current decode paths; the three OOBs look real and the guards are in the right places. Each new early return goes through the existing cleanup label, so h gets torn down with hdr_close() rather than hdr_free() — that's the exact thing PR #122 had to fix, and it's right here.

Four things worth a look before merge:

• The word_size != 2/4/8 rejection matches what Java's AbstractHistogram decode requires for the non-V2 cookie base. One nuance I can't verify from here: I believe Java masks the size nibble (sizeByte & 0xe) before comparing, so a cookie with nibble 3 decodes as word size 2 upstream but would now be rejected here. Worth checking against the Java source — likely academic, since nothing emits that.
• The V2 cap at MAX_BYTES_LEB128 * counts_len matches this library's own encoder bound (the encoded_len calculation in the V2 encode path), so it can't reject anything we produce. Java bounds payload_len against the input buffer's remaining bytes, which isn't knowable post-inflate here, so the encoder bound looks like the right substitute.
• Error codes are inconsistent for the same class of input: V1 negative payload_len → HDR_ENCODED_INPUT_TOO_LONG, V2 negative → EINVAL. Worth picking one.
• In a normal build the new tests only assert return codes; the per-PR ClusterFuzzLite job runs ASan over the fuzz targets, not ctest, so nothing runs these blobs under a sanitizer until fix: signed-shift overflow in hdr_calculate_bucket_config (UBSan, found by fuzzing) #145 lands. .clusterfuzzlite/hdr_decode_fuzzer.c already covers this exact entry point — adding the five blobs as a seed corpus would keep them under ASan (and UBSan in the weekly batch) regardless of merge order.

No struct layout or public signature change, so no SOVERSION implication — but it does change what the public decode API accepts on untrusted input, and the downstreams in this tracker do parse logs they didn't produce, so this probably wants an actual release rather than just sitting at head. The description also still only covers the V1/V2 fixes; the V0 word_size guard and the V2 ~2GB cap arrived in later commits.

I haven't built this or run the crafted blobs under ASan myself, so the before/after sanitizer output is the author's claim rather than something verified here.

[paulorsousa]

LGTM :)