This is an automated email from the ASF dual-hosted git repository. martin-g pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/avro-rs.git
commit 5f22c34b7da87420e4da114b348d32546c72fcc2 Author: Martin Grigorov <[email protected]> AuthorDate: Thu Aug 27 09:44:37 2026 +0300 fix: OCF block message count unbounded with zero-byte-consuming decode (#643) After decompression, if the declared count exceeds the number of bytes actually in the block (only possible when some datums are zero-width, e.g. schema "null"), the count itself must pass safe_collection_len::<Value> — i.e. the number of conjurable-from-nothing objects is bounded by the allocation budget (~16-21M at the 512MB default) instead of being trusted. Counts backed by at least one byte per object are unchanged, and the existing :209 guard still errors when a non-empty buffer yields a zero-consumption decode. Reported-by: Security scans --- avro/src/reader/block.rs | 110 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 109 insertions(+), 1 deletion(-) diff --git a/avro/src/reader/block.rs b/avro/src/reader/block.rs index c7343f7..a1b1003 100644 --- a/avro/src/reader/block.rs +++ b/avro/src/reader/block.rs @@ -162,7 +162,20 @@ impl<'r, R: Read> Block<'r, R> { // and replace `buf` with the new one, instead of reusing the same buffer. // We can address this by using some "limited read" type to decode directly // into the buffer. But this is fine, for now. - self.codec.decompress(&mut self.buf) + self.codec.decompress(&mut self.buf)?; + + // The declared object count is attacker-controlled header data + // and is not otherwise required to be backed by actual bytes: + // with a zero-width writer schema (e.g. "null") every object + // decodes without consuming input, so a ~80-byte file could + // declare 2^62 objects and make readers spin or OOM on + // collect(). A count larger than the decompressed block size + // is only possible for such zero-width datums; bound it by + // the allocation budget instead of trusting the header. + if self.message_count > self.buf.len() { + util::safe_collection_len::<Value>(self.message_count)?; + } + Ok(()) } Err(Details::ReadVariableIntegerBytes(io_err)) => { if let ErrorKind::UnexpectedEof = io_err.kind() { @@ -361,6 +374,101 @@ mod tests { use super::Block; use crate::error::Details; use crate::{Codec, Schema}; + use apache_avro_test_helper::TestResult; + + #[test] + fn avro_rs_643_huge_message_count_on_empty_block_is_rejected() -> TestResult { + // Block header claiming 2^62 objects with a block size of 0: with a + // zero-width writer schema (e.g. "null") each object decodes without + // consuming input, so the declared count must be bounded by the + // allocation budget instead of being trusted. + let mut data = Vec::new(); + crate::util::zig_i64(1i64 << 62, &mut data)?; + data.push(0x00); // block size 0 + data.extend([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]); // sync marker + + let mut block = Block::<'_, &[u8]> { + reader: &data[..], + buf: vec![], + buf_idx: 0, + message_count: 0, + marker: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16], + codec: Codec::Null, + writer_schema: Schema::Null, + schemata: vec![], + user_metadata: Default::default(), + names_refs: Default::default(), + human_readable: false, + }; + + let err = block.read_block_next().unwrap_err().into_details(); + // 2^62 * size_of::<Value>() overflows usize, so this trips the + // checked_mul overflow arm inside safe_collection_len. + assert!(matches!(err, Details::IntegerOverflow), "{err:?}"); + Ok(()) + } + + #[test] + fn avro_rs_643_over_budget_message_count_on_empty_block_is_rejected() -> TestResult { + // A count that multiplies cleanly by size_of::<Value>() but exceeds + // the allocation budget must trip the budget check itself, pinning + // the MemoryAllocation path alongside the overflow one above. + let budget = crate::util::DEFAULT_MAX_ALLOCATION_BYTES; + let count = budget / size_of::<crate::types::Value>() + 1; + + let mut data = Vec::new(); + crate::util::zig_i64(count as i64, &mut data)?; + data.push(0x00); // block size 0 + data.extend([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]); // sync marker + + let mut block = Block::<'_, &[u8]> { + reader: &data[..], + buf: vec![], + buf_idx: 0, + message_count: 0, + marker: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16], + codec: Codec::Null, + writer_schema: Schema::Null, + schemata: vec![], + user_metadata: Default::default(), + names_refs: Default::default(), + human_readable: false, + }; + + let err = block.read_block_next().unwrap_err().into_details(); + assert!(matches!(err, Details::MemoryAllocation { .. }), "{err:?}"); + + Ok(()) + } + + #[test] + fn avro_rs_643_small_message_count_on_empty_block_is_accepted() -> TestResult { + // The crate's own writer produces count > 0 with an empty block body + // for zero-width datums (e.g. schema "null"); those must keep working. + let null_values_count = 3; + let mut data = Vec::new(); + crate::util::zig_i64(null_values_count, &mut data)?; + data.push(0x00); // block size 0 + data.extend([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]); // sync marker + + let mut block = Block::<'_, &[u8]> { + reader: &data[..], + buf: vec![], + buf_idx: 0, + message_count: 0, + marker: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16], + codec: Codec::Null, + writer_schema: Schema::Null, + schemata: vec![], + user_metadata: Default::default(), + names_refs: Default::default(), + human_readable: false, + }; + + block.read_block_next()?; + assert_eq!(block.message_count, null_values_count as usize); + Ok(()) + } #[test] fn avro_rs_586_negative_block_size() {
