AVRO-4296: [python] Bound allocation when decoding length-prefixed values and collections

[iemejia]

What changes were proposed in this pull request?

When decoding an array or map, DatumReader.read_array/read_map used the block count read from the (potentially malformed or truncated) input directly as a loop counter, and the length-prefixed byte/string readers allocated from an unchecked length. A small payload could therefore declare a very large count or length and drive an unbounded allocation before any element bytes were present.

This bounds those allocations, mirroring the Java SDK's two-limit approach:

• Bytes-remaining check. Length-prefixed values and collection blocks are validated against the number of bytes actually remaining in the input (when the reader is seekable) before allocating, so a truncated payload fails fast instead of over-allocating.
• Zero-byte element cap. Element types whose minimum encoded size is zero (null, a zero-length fixed, or a record whose fields are all zero-byte) cannot be bounded by the bytes remaining, so their cumulative count is capped by a separate configurable limit (AVRO_MAX_COLLECTION_ITEMS). A structural cap of Integer.MAX_VALUE - 8 applies to all collections as defense in depth.

It also hardens related decode paths surfaced by this work:

• overlong / 64-bit-overflowing varints are rejected in read_long/skip_long;
• negative skips and out-of-range union/enum indices are rejected;
• sized-block skips validate their declared byte size;
• bytes_remaining() always restores the reader position and degrades gracefully on non-seekable readers.

Reading malformed or truncated input now fails fast with a clear AvroCollectionSizeException or InvalidAvroBinaryEncoding; valid data reads unchanged.

How was this patch tested?

• Extensive new unit tests in avro/test/test_io.py covering: array/map counts beyond the bytes remaining, the zero-byte-element cap (including null, zero-length fixed, all-zero-byte records, cumulative counts across blocks, negative and INT64_MIN block counts), the AVRO_MAX_COLLECTION_ITEMS override (valid/invalid/negative), varint bounds, sized-block skip validation, and non-seekable readers.
• Valid data (including arrays of null) continues to round-trip unchanged, and the existing enum-default forward-compatibility behavior is preserved.
• Full avro Python test suite passes across supported interpreters.

JIRA

• AVRO-4296

[Copilot]

Pull request overview

This PR hardens the Python Avro binary decoding path against malicious or truncated inputs that declare excessively large length/count prefixes, by validating declared sizes against bytes remaining for seekable inputs before allocating/iterating.

Changes:

• Add BinaryDecoder.bytes_remaining() and use it to pre-reject oversized read(n) requests (above a threshold) when the reader is seekable.
• Add minimum on-wire-size estimation for schemas and use it to validate array/map block counts in DatumReader against remaining bytes.
• Add unit tests covering oversized length prefixes, oversized collection block counts, and a non-false-positive case (array of null).

Reviewed changes

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

File	Description
lang/py/avro/io.py	Adds remaining-bytes introspection plus pre-checks for large length-prefixed reads and collection block count validation using per-element minimum sizes.
lang/py/avro/test/test_io.py	Adds targeted tests for the new available-bytes validation behavior in BinaryDecoder and DatumReader (including array-of-nulls).

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[iemejia]

This PR now also includes the collection block-count cap for [python], so it is the single complete fix for collection allocation DoS in this SDK. In addition to validating available bytes before allocating length-prefixed values, it bounds the number of array/map items per block:

• a heap-aware cap for zero-byte-element collections (e.g. array<null>), which otherwise bypass the available-bytes check because each element reads 0 bytes;
• a structural cap for all collections;
• bounded skip paths so projection/skip cannot loop unboundedly.

With this, the standalone collection-limit change for [python] (AVRO-4282, #3845) is redundant and is being closed as superseded by this PR.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[iemejia]

Preallocation (AVRO-4292 follow-up) evaluated — no change needed here. read_array/read_map build read_items/the dict via .append(), growing on demand rather than preallocating to the declared block count, so there is no up-front over-allocation to bound (unlike the Java/C++/C# readers, which sized a buffer to the count). The existing structural and zero-byte caps already bound the growth.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

lang/py/avro/io.py:1032

• read_map() uses len(read_items) as the "existing" element count for _ensure_collection_available(). Because maps can legally contain duplicate keys on the wire (which overwrite in the dict), len(read_items) can significantly undercount the number of key/value pairs actually decoded. This weakens the cumulative/structural cap and can allow an attacker to bypass the intended element-count limit by repeating keys across blocks.

        read_items: Dict[str, object] = {}
        # Map keys are strings (>= 1 byte length prefix) plus the value.
        min_bytes = 1 + _min_bytes_per_element(writers_schema.values)
        zero_byte_limit, structural_limit = _collection_limits()
        block_count = decoder.read_long()
        while block_count != 0:
            if block_count < 0:
                block_count = -block_count
                decoder.skip_long()
            self._ensure_collection_available(decoder, len(read_items), block_count, min_bytes, zero_byte_limit, structural_limit)
            for i in range(block_count):
                key = decoder.read_utf8()
                read_items[key] = self.read_data(writers_schema.values, readers_schema.values, decoder)
            block_count = decoder.read_long()

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[RyanSkraba]

Just curious -- what is the #: for? We don't use this anywhere else in the project.

[iemejia]

Good catch — #: is Sphinx/autodoc syntax that documents the attribute on the following line, but you're right the project doesn't use it anywhere else and doesn't render autodoc attribute docs, so it was just inconsistent. Switched all of these to plain # comments (and dropped the couple of :data: roles for the same reason).

[RyanSkraba]

Redundant with skip(n) also checking negative bytes -- remove or align error messages?

[iemejia]

Removed the redundant check — a negative length now falls through to skip(), which already rejects backward seeks, so there's a single enforcement point. (read() keeps its own check since it doesn't delegate to skip().)

[arib06]

Took a pass over this since it supersedes my #3876. The enum/union index hardening here is a superset of what I had, so #3876 is closed.

Checked out the branch, python3 -m unittest avro.test.test_io is green (186 tests). I confirmed the two things I would most expect to be off-by-one:

• the 10-byte varint cap in read_long/skip_long still round-trips the full 64-bit range (2**63-1 and -2**63 both encode to 10 bytes and decode correctly), and the shift == 63 and (b & 0x7E) guard correctly allows bit 63 while rejecting bits 64+.
• array<null> with a 200M block count raises AvroCollectionSizeException, and a huge declared count with a tiny payload raises InvalidAvroBinaryEncoding.

One thing worth considering before merge. _ensure_collection_available calls decoder.bytes_remaining() for every array/map block whose element type has a positive minimum size, no matter how small the declared count is, and bytes_remaining() does a tell + seek(0, SEEK_END) + tell + seek(pos) each time. On a real file object that is not free. Decoding 20k small array<int> values from a file:

with checks:                   0.137s
bytes_remaining stubbed None:  0.074s   (~84% overhead)

On BytesIO it barely shows, which is why the tests do not surface it, but the seek is per block against the underlying reader.

read() already avoids this with _MAX_UNCHECKED_READ, on the reasoning that a small read cannot over-allocate meaningfully. The same reasoning applies to a small block count. Gating just the bytes_remaining() call on a threshold recovers it:

if min_bytes_per_element > 0:
    if count > _MAX_UNCHECKED_COLLECTION:   # e.g. 1024
        remaining = decoder.bytes_remaining()
        if remaining is not None and count > remaining // min_bytes_per_element:
            raise ...
    if existing + count > structural_limit:  # keep unconditional, no seek
        raise ...

I tried that locally: back to 0.074s, and both attack payloads above are still rejected. It does not weaken the bound either, since with min_bytes > 0 a block of under-threshold elements still has to be paid for in real bytes on the wire, so total allocation stays bounded by the input size. The zero-byte path is untouched and keeps the tighter cumulative cap, which is the case where the bytes-remaining check could not bound anything anyway.

Minor, take it or leave it: _collection_limits() returns (parsed, parsed), so setting AVRO_MAX_COLLECTION_ITEMS to raise the zero-byte limit also lowers the structural cap from ~2.1B to that same value. It is documented, but the coupling runs in both directions and the name only suggests one. Might be worth a line in the docstring calling that out explicitly.

Neither is a blocker. LGTM otherwise.

[iemejia]

Thanks for the detailed benchmarking, @arib06. Applied both suggestions:

• _ensure_collection_available now only calls bytes_remaining() when the block count exceeds a _MAX_UNCHECKED_COLLECTION threshold (1024), mirroring _MAX_UNCHECKED_READ in read(); the structural cap stays unconditional with no seek. As you noted this doesn't weaken the bound — with min_bytes > 0 an under-threshold block still has to be paid for in real wire bytes, and the zero-byte path is untouched and keeps its tighter cumulative cap. Added a regression test for a small truncated collection to lock that in.
• Expanded the _collection_limits() docstring to call out explicitly that AVRO_MAX_COLLECTION_ITEMS pins both limits to the same value, so raising the zero-byte limit also lowers the structural cap (and how to avoid that by setting it above DEFAULT_MAX_COLLECTION_STRUCTURAL).

[Copilot]

Pull request overview

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

Suppressed comments (1)

lang/py/avro/io.py:470

• skip_bytes() only validates oversized lengths when bytes_remaining() returns a non-None value. However bytes_remaining() can return None even for a reader that still supports tell()/seek() (e.g. if seek(0, SEEK_END) fails as in the FailingEndStream test helper). In that case an attacker-controlled large length prefix can be skipped via seek() past EOF without error; if the skipped bytes/string field is the final field being skipped (e.g. during schema resolution), truncated input may be accepted instead of failing fast.

        n = self.read_long()
        if n > self._MAX_UNCHECKED_READ:
            remaining = self.bytes_remaining()
            if remaining is not None and n > remaining:
                raise avro.errors.InvalidAvroBinaryEncoding(f"Requested {n} bytes to skip, but only {remaining} remain.")

[RyanSkraba]

Cherry-picked to [branch-1.12|https://github.com/apache/avro/commit/c702676903c38680171f41fa95d4e5c56d01efff].