p2p: Lower initial allocation for Vec Decoder
What the developer wrote
Authored by Mitchell Bagot
p2p: 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 how much memory is pre-allocated when reading a variable-length list of items from a Bitcoin peer message. Previously the code could reserve up to roughly a quarter of a block's worth of elements based on an untrusted length value, which could let a malicious peer trick the program into reserving a large chunk of RAM. The new code caps the initial reservation to at most 8,000 bytes and lets the vector grow normally if more items actually arrive. It is a hardening change, not a fix for a known active attack.
Treat as a defensive hardening patch. Review whether 8,000 bytes is appropriate for all wrapped types and whether downstream code relies on any specific reservation behavior. No urgent security response is indicated by the commit itself.
Security signals we found
Untrusted length field drives initial memory allocation
Allocation cap reduced from block-scale to 8 KB
OOM/DoS hardening for P2P message decoding
No semantic change to decoded output; only memory reservation policy
Evidence from the diff
In p2p/src/consensus.rs the Vec
Changed components
p2p/src/consensus.rsVec<T> consensus_decode_from_finite_readerP2P message decoding pathInspect captured patch +6 / −7
diff --git a/p2p/src/consensus.rs b/p2p/src/consensus.rs
index 95aae1bd..b84a4ebd 100644
--- a/p2p/src/consensus.rs
+++ b/p2p/src/consensus.rs
@@ -80,15 +80,14 @@ macro_rules! impl_vec_wrapper {
r: &mut R,
) -> core::result::Result<$wrapper, bitcoin::consensus::encode::Error> {
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 =
- bitcoin::consensus::encode::MAX_VEC_SIZE / 4 / core::mem::size_of::<$type>();
- let mut ret = Vec::with_capacity(core::cmp::min(len as usize, max_capacity));
+ let max_init_capacity = 8000 / core::mem::size_of::<$type>();
+ let mut ret = Vec::with_capacity(core::cmp::min(len as usize, max_init_capacity));
for _ in 0..len {
ret.push(Decodable::consensus_decode_from_finite_reader(r)?);
}
Why this scored 41/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.