fix: bound array/map allocation by decoded element count by iemejia · Pull Request #581 · apache/avro-rs

iemejia · 2026-07-12T15:46:19Z

What

An array or map block is encoded as an element count followed by that many items. When decoding, items.reserve(len) allocates len * size_of::<Value>() bytes (Value is 56 bytes), but the count was only validated with safe_len, which bounds it as if it were a byte count.

As a result a tiny payload declaring a huge block count could drive a multi-gigabyte allocation far exceeding the configured max_allocation_bytes budget. At the safe_len ceiling this reaches ~28 GiB. Reproduced with a 5-byte payload (array<null>, block count 10_000_000) decoding into a 10M-element array (534 MiB) — exceeding the default 512 MiB budget while being accepted.

This affects:

both arrays and maps;
both zero-byte on-wire elements (null) and non-zero-byte elements — the reserve happens before any element is read;
and the zero-byte "flood" where the decode loop grows the collection unboundedly (zero-byte elements consume no input, so nothing else bounds the count).

How

Add util::safe_collection_len(total_items, item_size), which bounds the cumulative element count against max_allocation_bytes, accounting for the in-memory size of each decoded element (mirroring safe_len, which only covers byte-length allocations).
Apply it in the Schema::Array and Schema::Map decode loops before reserve, using size_of::<Value>() and size_of::<(String, Value)>() respectively, with a checked_add guard for the running total.

This caps both the up-front reservation and the total number of decoded elements. The limit stays configurable through max_allocation_bytes (the default 512 MiB budget yields ~9.6M Values, close to the item caps used by the other Avro SDKs).

bytes/string length-prefixed values are already bounded by safe_len; since decode operates on a bare Read with no known remaining size, that budget cap is the appropriate guard (equivalent to the non-seekable path in the other SDKs).

Tests

New: huge array<null>, array<long>, and map block counts are rejected instead of allocating; a small array<null> still decodes.
cargo test -p apache-avro (564 lib tests) passes; cargo fmt --check and cargo clippy are clean.

An array or map block is encoded as an element count followed by that many items. When decoding, `items.reserve(len)` allocates `len * size_of::<Value>()` bytes (Value is 56 bytes), but the count was only validated with `safe_len`, which bounds it as if it were a byte count. So a tiny payload declaring a huge block count could drive a multi-gigabyte allocation far exceeding the configured `max_allocation_bytes` budget (up to ~28 GiB at the safe_len ceiling), and a count of zero-byte on-wire elements (e.g. `null`) could grow the collection unboundedly since such elements consume no input. Add `util::safe_collection_len`, which bounds the cumulative element count against the allocation budget accounting for the in-memory size of each element, and apply it in the array and map decode loops before reserving. This caps both the up-front reservation and the total number of decoded elements, covering zero-byte and non-zero-byte element types alike. The limit stays configurable through `max_allocation_bytes`. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

This PR hardens Avro decoding against maliciously large array/map block counts that could trigger excessive in-memory allocations (especially via reserve(len)), by bounding decoded element counts against the configured allocation budget.

Changes:

Add util::safe_collection_len(total_items, item_size) to enforce an allocation-budget cap for cumulative decoded collection sizes (arrays/maps).
Apply the new guard in Schema::Array and Schema::Map decode loops before calling reserve.
Add decode tests ensuring huge array/map counts are rejected while small valid inputs still decode.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
avro/src/util.rs	Introduces `safe_collection_len` to bound collection element-count driven allocations against `max_allocation_bytes`.
avro/src/decode.rs	Uses `safe_collection_len` in array/map decoding prior to `reserve`, and adds regression tests for huge block counts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

decode_seq_len already rejects a block count whose absolute value is i64::MIN via checked_neg (which returns None, yielding Details::IntegerOverflow), so no code change is needed. Add a regression test decoding the 10-byte i64::MIN varint as a negative block count and asserting it is rejected, for parity with the negation-overflow coverage in the other Avro SDKs. Assisted-by: GitHub Copilot:claude-opus-4.8

- safe_collection_len: use checked_mul instead of saturating_mul so a usize::MAX allocation budget cannot mask a multiplication overflow (which would let reserve() hit a capacity-overflow panic); return Details::IntegerOverflow on overflow. - tests: build block-count varints via the crate's zig_i64 encoder rather than a duplicated zigzag/varint implementation. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

iemejia · 2026-07-12T17:40:53Z

+                    .checked_add(len)
+                    .ok_or(Details::IntegerOverflow)?;
+                safe_collection_len(total, std::mem::size_of::<Value>())?;
                items.reserve(len);


Fixed in 2aa1ba1: array decoding now uses reserve_exact(len), so the reservation matches the checked element count rather than reserve's amortized (larger) capacity.

iemejia · 2026-07-12T17:40:54Z

+/// [`safe_len`] guards byte-length allocations, but an array or map block is a
+/// count of elements, and reserving capacity for `n` elements allocates
+/// `n * item_size` bytes (`item_size` being the in-memory size of a decoded
+/// element). A malicious or truncated input can declare a huge block count in a


Fixed in 2aa1ba1: the doc now says reserving allocates at least n * item_size bytes and notes a collection may over-allocate for growth/metadata, so it's a lower-bound estimate.

…stimate Addresses review: - Array decoding uses reserve_exact so the reservation matches the checked element count, rather than reserve's amortized (larger) capacity which could request more than max_allocation_bytes across multiple blocks. - Clarify in the safe_collection_len doc and the map decode comment that the per-element size is a lower-bound estimate: HashMap::reserve (and Vec growth) can over-allocate for metadata/spare capacity, so the budget is enforced approximately for maps (there is no HashMap::reserve_exact). Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Document that safe_collection_len uses item_size.max(1), so a zero-sized element type is treated as one byte and the reported `desired` allocation is total_items * max(item_size, 1). This matches the implementation and clarifies the error's `desired` interpretation. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

iemejia mentioned this pull request Jul 12, 2026

fix: bound decompressed size to guard against decompression bombs #582

Open

iemejia requested a review from Copilot July 12, 2026 15:57

Copilot started reviewing on behalf of iemejia July 12, 2026 15:58 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

Comment thread avro/src/util.rs

Comment thread avro/src/decode.rs

iemejia added 2 commits July 12, 2026 18:44

iemejia requested a review from Copilot July 12, 2026 16:55

Copilot started reviewing on behalf of iemejia July 12, 2026 16:56 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

iemejia requested a review from Copilot July 12, 2026 18:09

Copilot started reviewing on behalf of iemejia July 12, 2026 18:09 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

Comment thread avro/src/util.rs

iemejia requested a review from Copilot July 12, 2026 18:22

Copilot started reviewing on behalf of iemejia July 12, 2026 18:22 View session

Copilot AI reviewed Jul 12, 2026

View reviewed changes

iemejia requested a review from martin-g July 12, 2026 18:28

martin-g approved these changes Jul 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: bound array/map allocation by decoded element count#581

fix: bound array/map allocation by decoded element count#581
iemejia wants to merge 5 commits into
apache:mainfrom
iemejia:fix/bound-collection-block-count-allocation

iemejia commented Jul 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

iemejia Jul 12, 2026

Uh oh!

iemejia Jul 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

iemejia commented Jul 12, 2026

What

How

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

iemejia Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

iemejia Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants