GHSA-mc57-h6j3-3hmv iskorotkov/avro: Integer Overflow in Decoder

• Go github.com/iskorotkov/avro/v2: < 2.33.0 → fixed in 2.33.0

Integer Overflow in Avro Decoder Summary Several Avro decoder paths read attacker-controlled 64-bit values from the wire format and either narrowed them to platform-sized `int` before bounds-checking, or summed them with overflow-prone signed-`int` arithmetic. On 32-bit targets (`GOARCH=386`, `arm`, `mips`, `wasm`, etc.), the truncation paths can silently bypass byte-slice limits, select the wrong union branch, or hit the OCF negative-`make` panic via wrap. Three sub-issues are not 32-bit-specific: cumulative-size arithmetic overflow in `arrayDecoder.Decode` / `mapDecoder.Decode` / `mapDecoderUnmarshaler.Decode` (wraps at `math.MaxInt64` on amd64 / arm64 and bypasses `MaxSliceAllocSize` / `MaxMapAllocSize`), `math.MinInt` negation in block-header handling, and `make([]byte, size)` with a negative size in OCF block reads — all three panic or bypass caps on any platform, giving an attacker a denial-of-service primitive there. Exploitation requires only an untrusted Avro stream. No primitives reach beyond denial-of-service on current code paths; see the union-index discussion below for a caveat. Description Six call sites in the decoder accepted `int64` values from the Avro wire format and converted to `int` before validation. On a 32-bit build any wire value with magnitude `≥ 2³¹` truncates and the post-conversion value bears no useful relationship to the original. A value of `(1 limit`. On amd64 / arm64 the running total wraps at `math.MaxInt64`; the post-wrap negative value passes the `> limit` check, and the decoder proceeds. Regression test: `TestDecoder_ArrayMultiBlockExceedsMaxInt` uses `math.MaxInt − 2` for the second block's count and a `MaxSliceAllocSize` of 13 to demonstrate this on amd64. The Avro block-count field is a signed `long` on the wire, so block counts up to `math.MaxInt64` are admissible — there is no implicit 2³¹ ceiling. • `ReadBlockHeader()` returns the absolute value of negative block lengths; the negation is unsafe for `math.MinInt`, which on every platform panics on overflow. • `ocf/ocf.go readBlock()` passes the decoded block size directly to `make([]byte, size)`. A negative wire value panics on every platform; on 32-bit, values `> MaxInt32` additionally panic via the narrowing path. Affected components | File | Function(s) | Bug class | Platforms | |------|-------------|-----------|-----------| | `reader.go` | `ReadBlockHeader` — narrowing | Narrowing | 32-bit | | `reader.go` | `ReadBlockHeader` — `-math.MinInt` | Signed overflow (CWE-191) | all | | `reader.go` | `readBytes` (via `Reader.ReadBytes`, `Reader.ReadString`) | Narrowing | 32-bit | | `reader_skip.go` | `SkipString`, `SkipBytes` (and OCF skip path) | Narrowing | 32-bit | | `codec_array.go` | `arrayDecoder.Decode` | Cumulative-size arithmetic overflow (CWE-190) | all | | `codec_map.go` | `mapDecoder.Decode`, `mapDecoderUnmarshaler.Decode` | Cumulative-size arithmetic overflow (CWE-190) | all | | `ocf/ocf.go` | `skipToEnd`, `readBlock` — narrowing | Narrowing | 32-bit | | `ocf/ocf.go` | `readBlock` — negative `make([]byte, …)` | Unchecked-negative (CWE-1284) | all | | `reader_generic.go` | union-type index decoding in `Reader.ReadNext` | Narrowing, possible wrong-branch selection | 32-bit | PR #9 (commit `bed99b3`) covered `ReadBlockHeader`, the cumulative checks in array/map codecs, and the skip helpers. The completeness pass (commit `e1a570f`) covered the union index, `readBytes`, and OCF `readBlock`, and added a 32-bit CI job. Note: the typed-codec union decoder in `codec_union.go` (`getUnionSchema` → `Reader.ReadInt`) is not affected by the union-index narrowing — `ReadInt` returns `int32`, no narrowing occurs. The narrowing is specific to `Reader.ReadNext` in the generic decode path (reached via `Unmarshal` into `any` / `map[string]any`). Technical details 1. Block-header narrowing and `MinInt` negation. `ReadBlockHeader()` returned wire-format `int64` values through narrower operations; on 32-bit, large positives truncated. Negating `math.MinInt` to convert a negative block-count signal into a positive size is undefined-on-overflow, and on every platform `-MinInt` panics on overflow when used in subsequent arithmetic. The fix reads into a `*64`-suffixed local, range-checks against `MinInt32`/`MaxInt32` (or `MinInt`/`MaxInt` as appropriate), and narrows after validation. 2. Cumulative array and map size overflow (all platforms). `arrayDecoder.Decode`, `mapDecoder.Decode`, and `mapDecoderUnmarshaler.Decode` summed attacker-controlled block lengths using overflow-prone addition; cumulative size could wrap before reaching the configured limit. On amd64 with `MaxSliceAllocSize = 13`, block 1 of 3 elements, block 2 of `math.MaxInt − 2` elements: the pre-fix `size += int(l)` wraps to `math.MinInt`, then `MinInt > 13` is false, so the check passes and the decoder proceeds. The fix uses subtraction-safe comparisons (`l > limit - size` rather than `size + l > limit`), which is overflow-immune. 3. Skip-length truncation. `SkipString`, `SkipBytes`, and the OCF skip helper now route through `SkipNBytesInt64()`, which keeps the length as `int64` and range-checks before any narrowing. 4. Byte-slice length truncation. A wire-format length such as `(1 MaxInt32` additionally panics via the 32-bit narrowing path. The fix validates the size is in `[0, MaxByteSliceSize]` before narrowing. Fixed behavior Both commits apply the same pattern across every site: 1. Read the wire value into an `int64`-typed local. 2. Range-check upper and lower bounds before narrowing. 3. Compare cumulative limits using subtraction-safe arithmetic. 4. Route skip operations through `SkipNBytesInt64()`. 5. Return descriptive errors using the consistent `"value is too big"` / `"value is too small"` wording. 6. Cast to `int` only after validation succeeds. CI: a `test-386` job runs the suite under `GOARCH=386` with `CGO_ENABLED=0` (`-race` is amd64/arm64-only). Three tests with untyped `2147483648` consta…