bitcoin: Lower initial allocation for Vec Decoder
What the developer wrote
Authored by Mitchell Bagot
bitcoin: Lower initial allocation for Vec Decoder
In the implementation of consensus_decode_from_finite_reader for Vec<T>,
the initial vector capacity is calculated as 4_000_000 / 4 / sizeof<T>.
This is an arbitrary value that doesn't provide any specific benefit,
per discussion in #4797. Fedimint has a similar implementation of the
function using different, lower values.
Change the initial allocation capacity to match Fedimint's.
What changed, and why it matters
This commit reduces the initial memory allocation when decoding a variable-length list (Vec) from Bitcoin protocol data. Previously, the code could reserve a large chunk of memory based on an untrusted length value, which could be abused to waste or exhaust memory. The new code caps the initial reservation to roughly 8,000 bytes worth of elements and grows the vector only as items are actually read. This is a hardening change against denial-of-service via maliciously large length fields.
Treat as a security hardening patch. Review whether other Decodable collection types (e.g., Box<[T]>, custom containers) use similar upfront allocation based on untrusted lengths and apply consistent caps. Consider adding regression tests with oversized compact-size length fields.
Security signals we found
Untrusted length value used for memory allocation
Initial vector capacity capped to limit memory reservation
OOM protection mentioned in code comment
Change motivated by denial-of-service hardening
Evidence from the diff
In bitcoin/src/consensus/encode.rs, the Vec
Changed components
bitcoin/src/consensus/encode.rsVec<T>::consensus_decode_from_finite_readerInspect captured patch +6 / −6
diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs
index 90e44d0c..2a0906f1 100644
--- a/bitcoin/src/consensus/encode.rs
+++ b/bitcoin/src/consensus/encode.rs
@@ -502,14 +502,14 @@ impl<T: Decodable + 'static> Decodable for Vec<T> {
unsafe { Ok(mem::transmute::<Vec<u8>, Self>(bytes)) }
} else {
let len = r.read_compact_size()?;
- // Do not allocate upfront more items than if the sequence of type
- // occupied roughly quarter a block. This should never be the case
- // for normal data, but even if that's not true - `push` will just
- // reallocate.
+ // Limit the initial vec allocation to at most 8,000 bytes, which is
+ // sufficient for most use cases. We don't allocate more space upfront
+ // than this, since `len` is an untrusted allocation capacity. If the
+ // vector does overflow the initial capacity `push` will just reallocate.
// Note: OOM protection relies on reader eventually running out of
// data to feed us.
- let max_capacity = MAX_VEC_SIZE / 4 / mem::size_of::<T>();
- let mut ret = Self::with_capacity(cmp::min(len as usize, max_capacity));
+ let max_init_capacity = 8000 / mem::size_of::<T>();
+ let mut ret = Self::with_capacity(cmp::min(len as usize, max_init_capacity));
for _ in 0..len {
ret.push(Decodable::consensus_decode_from_finite_reader(r)?);
}
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.