Merge rust-bitcoin/rust-bitcoin#6763: bip152: validate prefill count before allocating short IDs
What the developer wrote
Authored by Andrew Poelstra
Merge rust-bitcoin/rust-bitcoin#6763: bip152: validate prefill count before allocating short IDs
7f80030897591c1d3bcf5023fe24d78115034403 bip152: test excessive prefill indexes (racequite)
cf94409eb486fd20e404acca67e87ccec6f1ebb4 bip152: validate prefill count before allocating short IDs (racequite)
Pull request description:
`HeaderAndShortIds::from_block` documents that invalid prefill indexes return `Error::InvalidPrefill`. However, it calculated the short ID capacity using unchecked subtraction before validating the input.
When the number of prefill indexes exceeds the number of transactions, this subtraction panics with overflow checks enabled. Other build configurations may wrap the subtraction and attempt an invalid or excessively large allocation.
Use `checked_sub` to return `Error::InvalidPrefill` before allocating the short ID vector. Add a regression test covering a single-transaction block with too many prefill indexes.
ACKs for top commit:
apoelstra:
ACK 7f80030897591c1d3bcf5023fe24d78115034403; successfully ran local tests
Tree-SHA512: 22d77e24df00332016107c482ae68c16fcdea387fa5fd4e5675d2e90c9067a4c1a961b6a9523bc58c99260f1011ae12e30e034881c7c2ccdb45f68972b948a9c
What changed, and why it matters
This change fixes a panic/crash bug in Bitcoin compact block (BIP152) encoding. When a caller asked to 'prefill' more transactions than actually exist in the block, the code subtracted the two numbers without checking, which could cause the program to panic (in safe builds) or try to allocate a nonsensical amount of memory (in other builds). The patch validates the count first and returns a proper error instead.
Merge the patch; it is a small, defensive fix with a regression test. Review other similar `with_capacity` arithmetic in the P2P module for the same pattern.
Security signals we found
Integer underflow in capacity calculation
Panic / denial-of-service via malformed input
Potential uncontrolled memory allocation
Violation of documented error contract
Missing input validation before resource allocation
Evidence from the diff
In HeaderAndShortIds::from_block, the short ID vector capacity was computed as block.transactions().len() - prefill.len() before validating that prefill.len() <= block.transactions().len(). With Rust overflow checks enabled this panics; in unchecked builds it wraps and feeds a huge value to Vec::with_capacity, risking an OOM/panic. The patch uses checked_sub(...).ok_or(Error::InvalidPrefill)? so the documented error is returned before allocation. A regression test is added for a single-transaction block with prefill indexes [1, 2].
Changed components
p2p/src/bip152.rsHeaderAndShortIds::from_blockBIP152 compact block serializationInspect captured patch +16 / −1
### p2p/src/bip152.rs
@@ -313,10 +313,13 @@ impl HeaderAndShortIds {
return Err(Error::UnknownVersion);
}
+ let short_id_capacity =
+ block.transactions().len().checked_sub(prefill.len()).ok_or(Error::InvalidPrefill)?;
+
let siphash_keys = ShortId::calculate_siphash_keys(block.header(), nonce);
let mut prefilled = Vec::with_capacity(prefill.len() + 1); // +1 for coinbase tx
- let mut short_ids = Vec::with_capacity(block.transactions().len() - prefill.len());
+ let mut short_ids = Vec::with_capacity(short_id_capacity);
let mut last_prefill = 0;
for (idx, tx) in block.transactions().iter().enumerate() {
// Check if we should prefill this tx.
@@ -892,6 +895,18 @@ mod test {
assert_eq!(idxs, [0, 1]);
}
+ #[test]
+ fn too_many_prefill_indexes_return_error_instead_of_panicking() {
+ let block = dummy_block();
+ let block = Block::new_unchecked(*block.header(), vec![block.transactions()[0].clone()])
+ .assume_checked(None);
+
+ assert_eq!(
+ HeaderAndShortIds::from_block(&block, 42, 2, &[1, 2]),
+ Err(Error::InvalidPrefill)
+ );
+ }
+
#[test]
fn compact_block_vector() {
// Tested with Elements implementation of compact blocks.Why this scored 57/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.