This is an automated email from the ASF dual-hosted git repository. martin-g pushed a commit to branch bound-allocated-bytes-for-fixed-schema-size in repository https://gitbox.apache.org/repos/asf/avro-rs.git
commit e51c6d7d87272aaf2fe12c887e35d46ae5005a84 Author: Martin Tzvetanov Grigorov <[email protected]> AuthorDate: Tue Aug 25 14:47:52 2026 +0300 fix: Fixed-schema decode allocates attacker-chosen size with no budget check Cap allocations to max_allocation_bytes when decoding OCF / deserializing schemas Reported-by: Security scans --- avro/src/decode.rs | 25 ++++++++++++++++++++++++- avro/src/schema/mod.rs | 23 +++++++++++++++++++++++ avro/src/schema/parser.rs | 9 +++++++-- avro/src/serde/deser_schema/mod.rs | 30 +++++++++++++++++++++++++++++- 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/avro/src/decode.rs b/avro/src/decode.rs index d78da71..d488356 100644 --- a/avro/src/decode.rs +++ b/avro/src/decode.rs @@ -226,7 +226,7 @@ pub(crate) fn decode_internal<R: Read, S: Borrow<Schema>>( } } Schema::Fixed(FixedSchema { size, .. }) => { - let mut buf = vec![0u8; *size]; + let mut buf = vec![0u8; safe_len(*size)?]; reader .read_exact(&mut buf) .map_err(|e| Details::ReadFixed(e, *size))?; @@ -487,6 +487,29 @@ mod tests { Ok(()) } + #[test] + fn avro_rs_640_test_decode_fixed_size_above_budget_is_rejected() -> TestResult { + use crate::schema::Name; + + // The schema (and with it the fixed size) can be attacker-supplied + // via an OCF header; the decoder must refuse to allocate more than + // the budget, before reading a single payload byte. + let schema = Schema::Fixed( + FixedSchema::builder() + .name(Name::new("huge")?) + .size(usize::MAX / 2) + .build(), + ); + let mut input: &[u8] = &[0u8; 4]; + let result = decode(&schema, &mut input); + assert!( + result.is_err(), + "a fixed size larger than the allocation budget must be rejected, got {result:?}" + ); + + Ok(()) + } + #[test] fn test_decode_map_without_size() -> TestResult { let mut input: &[u8] = &[0x02, 0x08, 0x74, 0x65, 0x73, 0x74, 0x02, 0x00]; diff --git a/avro/src/schema/mod.rs b/avro/src/schema/mod.rs index c7a9cb9..fe3d72e 100644 --- a/avro/src/schema/mod.rs +++ b/avro/src/schema/mod.rs @@ -1203,6 +1203,7 @@ fn field_ordering_position(field: &str) -> Option<usize> { #[cfg(test)] mod tests { use super::*; + use crate::util::{DEFAULT_MAX_ALLOCATION_BYTES, max_allocation_bytes}; use crate::writer::datum::GenericDatumWriter; use crate::{error::Details, rabin::Rabin, reader::datum::GenericDatumReader}; use apache_avro_test_helper::{ @@ -1217,6 +1218,28 @@ mod tests { assert!(Schema::parse_str("invalid").is_err()); } + #[test] + fn avro_rs_640_test_fixed_size_above_allocation_limit_is_rejected_at_parse() -> TestResult { + // A fixed schema's size is an allocation directive for decoders, so + // parsing must bound it ("schema parsing is safe" is a stated + // contract) instead of letting a hostile schema declare a + // terabyte-scale allocation. + let min_disallowed = max_allocation_bytes(DEFAULT_MAX_ALLOCATION_BYTES) + 1; + let result = Schema::parse_str(&format!( + r#"\{{"type": "fixed", "name": "huge", "size": {min_disallowed}}}"# + )); + assert!(result.is_err(), "expected a parse error, got {result:?}"); + + // A reasonable size must still parse. + let schema = Schema::parse_str(r#"{"type": "fixed", "name": "ok", "size": 16}"#)?; + assert!(matches!( + schema, + Schema::Fixed(FixedSchema { size: 16, .. }) + )); + + Ok(()) + } + #[test] fn test_primitive_schema() -> TestResult { assert_eq!(Schema::Null, Schema::parse_str(r#""null""#)?); diff --git a/avro/src/schema/parser.rs b/avro/src/schema/parser.rs index f84766f..98404e9 100644 --- a/avro/src/schema/parser.rs +++ b/avro/src/schema/parser.rs @@ -22,7 +22,7 @@ use crate::schema::{ SchemaKind, UnionSchema, UuidSchema, }; use crate::types; -use crate::util::MapHelper; +use crate::util::{MapHelper, safe_len}; use crate::validator::validate_enum_symbol_name; use crate::{AvroResult, Error}; use log::{debug, error, warn}; @@ -772,6 +772,11 @@ impl Parser { .ok_or_else(|| Details::GetFixedSizeFieldPositive(size.clone())), None => Err(Details::GetFixedSizeField), }?; + // A fixed schema's size directs decoders to allocate that many bytes, + // and the schema itself may be attacker-supplied (e.g. an OCF header). + // Always bound it before allocating. + let size = usize::try_from(size).map_err(|e| Details::ConvertU64ToUsize(e, size))?; + let size = safe_len(size)?; let fully_qualified_name = Name::parse(complex, enclosing_namespace)?; let aliases = @@ -781,7 +786,7 @@ impl Parser { name: fully_qualified_name.clone(), aliases: aliases.clone(), doc, - size: size as usize, + size, attributes: self.get_custom_attributes(complex, &["size"]), }); diff --git a/avro/src/serde/deser_schema/mod.rs b/avro/src/serde/deser_schema/mod.rs index 45287a3..4ebfa25 100644 --- a/avro/src/serde/deser_schema/mod.rs +++ b/avro/src/serde/deser_schema/mod.rs @@ -24,7 +24,7 @@ use crate::{ decode::decode_len, error::Details, schema::{DecimalSchema, InnerDecimalSchema, Name, UnionSchema, UuidSchema}, - util::{zag_i32, zag_i64}, + util::{safe_len, zag_i32, zag_i64}, }; mod block; @@ -190,6 +190,10 @@ impl<'s, 'r, R: Read, S: Borrow<Schema>> SchemaAwareDeserializer<'s, 'r, R, S> { /// /// This does not check the current schema. fn read_bytes(&mut self, length: usize) -> Result<Vec<u8>, Error> { + // `length` may be schema-declared rather than wire-declared (e.g. a + // fixed size from an attacker-supplied OCF writer schema); always + // bound it before allocating. + let length = safe_len(length)?; let mut buf = vec![0; length]; self.reader .read_exact(&mut buf) @@ -795,6 +799,30 @@ mod tests { Ok(()) } + #[test] + fn avro_rs_640_fixed_size_above_allocation_limit_is_rejected() -> TestResult { + use crate::schema::{FixedSchema, Name}; + + // The serde path must bound schema-declared fixed sizes exactly like + // the Value path does. + let schema = Schema::Fixed( + FixedSchema::builder() + .name(Name::new("huge")?) + .size(usize::MAX / 2) + .build(), + ); + let data = [0u8; 4]; + let result = GenericDatumReader::builder(&schema) + .build()? + .read_deser::<ByteBuf>(&mut &data[..]); + assert!( + result.is_err(), + "a fixed size larger than the allocation budget must be rejected, got {result:?}" + ); + + Ok(()) + } + #[test] fn avro_3955_decode_enum() -> TestResult { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
