[
https://issues.apache.org/jira/browse/AVRO-4283?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095743#comment-18095743
]
Ismaël Mejía commented on AVRO-4283:
------------------------------------
h3. Acceptance criteria / edge cases for the decompressed-size bound
While implementing the per-SDK subtasks, please ensure the following are
covered (and verify the already-done Java (AVRO-4247) and Rust (avro-rs #582)
impls satisfy them):
# *Enforce incrementally — never trust a declared/frame size.* snappy declares
the uncompressed length up-front; zstd exposes ZSTD_getFrameContentSize
(attacker-controlled, or ZSTD_CONTENTSIZE_UNKNOWN/ERROR); deflate/bzip2/xz have
no reliable declared size. The bound must be enforced *during* the
inflate/streaming decode loop, not by pre-trusting a header value — a "check
declared size then allocate" implementation is itself bomb-able.
# *Boundary conditions.* decompressed size == limit passes, limit+1 fails; also
handle size-0 and empty compressed blocks.
# *Snappy −4 CRC underflow.* Avro's snappy framing appends a 4-byte CRC; guard
the length − 4 subtraction against underflow (cf. the BadSnappyLength /
checked_sub(4) case fixed in avro-rs #582). Every SDK's snappy path needs this.
# *xz/lzma decoder memory (dictionary) limit.* This is separate from the
output-size cap: a malicious xz header can request a huge dictionary and OOM
the decoder before emitting any output. Set a decoder memlimit in addition to
bounding output.
# *Default + config consistency.* Default the limit to 200MB in every SDK
(mirroring Java's org.apache.avro.limits.decompress.maxLength) and expose an
equivalent tunable. Agree on the knob name (e.g. env
AVRO_MAX_DECOMPRESSED_LENGTH vs. a per-SDK property) and document it alongside
AVRO_MAX_COLLECTION_ITEMS.
# *Error taxonomy.* Reject an over-limit block with a distinct, bounded error
("exceeds decompressed limit") separate from "corrupt/truncated" — never a
generic OOM or crash.
# *Bound the compressed block read too.* The data-file reader allocates the
compressed block buffer from the header-declared block size (e.g. Java
DataFileStream: new byte[blockSize] guarded only by <= Integer.MAX_VALUE; a
few-byte file can force a ~2GB allocation). This must be bounded (against
bytes-available, capped when unknown) in the same PR, or the bomb OOMs before
decompression even runs. This also protects the null-codec case.
# *Per-block freeing.* Confirm decompressed blocks are released between
iterations so the per-block cap is sufficient and no reader retains them.
Items 1–8 also serve as the regression-test matrix: each should have a test
that fails with a bounded, well-defined error rather than exhausting memory.
> Enforce a maximum decompressed block size across language SDKs
> --------------------------------------------------------------
>
> Key: AVRO-4283
> URL: https://issues.apache.org/jira/browse/AVRO-4283
> Project: Apache Avro
> Issue Type: Improvement
> Components: c, c++, csharp, javascript, perl, php, python, ruby
> Affects Versions: 1.11.5, 1.12.1
> Reporter: Ismaël Mejía
> Assignee: Ismaël Mejía
> Priority: Major
> Fix For: 1.12.2
>
>
> When reading an Avro container (data) file, each block is decompressed
> according to the file's codec (deflate, bzip2, snappy, xz, zstandard).
> Several SDKs read the compressed block into memory and decompress into an
> output buffer that grows (or is sized from an attacker-declared length)
> without any upper bound. A block with a very high compression ratio can
> therefore expand to an amount of memory far larger than the input, leading to
> excessive allocation on malformed or hostile input.
> The Java SDK addressed this in AVRO-4247 by enforcing a maximum decompressed
> size in every codec, defaulting to 200MB and configurable via
> {{org.apache.avro.limits.decompress.maxLength}}. This umbrella tracks
> bringing the C, C++, C#, JavaScript, Perl, PHP, Python, and Ruby SDKs in line
> with that behavior so all SDKs bound decompression output consistently.
> h3. Proposed approach
> * Enforce a configurable maximum decompressed size while
> inflating/decompressing each data-file block, across all codecs.
> * Reject a block whose decompressed output would exceed the limit with a
> clear error instead of continuing to allocate.
> * Default the limit to a runtime-safe value; allow it to be tuned per SDK
> (mirroring the Java property).
> * Add regression tests: a block that decompresses to more than the limit must
> fail with a bounded, well-defined error rather than exhausting memory.
> An audit of the current decompression paths (for reference in the subtasks):
> * C: {{lang/c/src/codec.c}} (deflate doubles the output buffer; snappy
> allocates the attacker-declared uncompressed length; lzma doubles), driven
> from {{lang/c/src/datafile.c}}.
> * C++: {{lang/c++/impl/DataFile.cc}} (deflate/snappy) and
> {{lang/c++/impl/ZstdDecompressWrapper.cc}} (zstd trusts the frame content
> size). The existing {{BoundedInputStream}} bounds only the compressed input.
> * C#: {{lang/csharp/src/apache/main/File/DataFileReader.cs}} and the codec
> classes (deflate/snappy/zstandard/bzip2/xz), all via {{Stream.CopyTo}} into
> an unbounded {{MemoryStream}}.
> * JavaScript: {{lang/js/lib/files.js}} (deflate via {{zlib.inflateRaw}} with
> no {{maxOutputLength}}).
> * Perl: {{lang/perl/lib/Avro/DataFileReader.pm}} (a {{block_max_size}}
> attribute exists but is never enforced).
> * PHP: {{lang/php/lib/DataFile/AvroDataIOReader.php}}
> (deflate/zstandard/snappy/bzip2).
> * Python: {{lang/py/avro/codecs.py}} (deflate/bzip2/snappy/zstandard).
> * Ruby: {{lang/ruby/lib/avro/data_file.rb}} (deflate/snappy/zstandard).
> Subtasks: one per SDK (below).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
