iemejia commented on code in PR #581:
URL: https://github.com/apache/avro-rs/pull/581#discussion_r3566790866
##########
avro/src/decode.rs:
##########
@@ -241,6 +241,16 @@ pub(crate) fn decode_internal<R: Read, S: Borrow<Schema>>(
break;
}
+ // Bound the cumulative element count against the allocation
+ // budget before reserving: `reserve(len)` allocates
+ // `len * size_of::<Value>()` bytes, and a tiny block count
could
+ // otherwise drive a huge allocation. This also caps zero-byte
+ // on-wire elements (e.g. `null`), which consume no input.
+ let total = items
+ .len()
+ .checked_add(len)
+ .ok_or(Details::IntegerOverflow)?;
+ safe_collection_len(total, std::mem::size_of::<Value>())?;
items.reserve(len);
Review Comment:
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.
##########
avro/src/util.rs:
##########
@@ -184,6 +184,37 @@ pub(crate) fn safe_len(len: usize) -> AvroResult<usize> {
}
}
+/// Bound the cumulative number of elements a collection (array or map) may
hold.
+///
+/// [`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
Review Comment:
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.
##########
avro/src/decode.rs:
##########
@@ -263,6 +273,13 @@ pub(crate) fn decode_internal<R: Read, S: Borrow<Schema>>(
break;
}
+ // See the array case above; a map entry additionally carries a
+ // String key, so bound by the size of a `(String, Value)`
pair.
Review Comment:
Fixed in 2aa1ba1: the map comment now states this is a lower-bound estimate
(`HashMap::reserve` also allocates table metadata and spare capacity, and there
is no `HashMap::reserve_exact`), so the budget is enforced approximately for
maps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
