← Watch feed
Merge rust-bitcoin/rust-bitcoin#6919: Sanitize serde size hints before allocating
Public commit record
100/100 · Strong
What the developer wrote
Authored by Andrew Poelstra
Merge rust-bitcoin/rust-bitcoin#6919: Sanitize serde size hints before allocating
f7c0ee459ed33fdade8977392f31bd8793e59d1a primitives: test witness size hint not trusted (satsfy (Renato Britto))
c48822a119e578d7a78ff8a16e6629d56cbec5db primitives: cap witness serde size hint for alloc (satsfy (Renato Britto))
91ff405a6318dac5051ee77c20e555ee7fdf33ff units: test serde size hints not trusted (satsfy (Renato Britto))
d4f03724d3b5fbaf073bf033361a65617f5ad71f units: cap serde size hints before preallocating (satsfy (Renato Britto))
803aff152959368852dd6bfd7f390ad5dff97833 internals: add cautious serde size hint (satsfy (Renato Britto))
Pull request description:
Close https://github.com/project-loupe/audit-rust-bitcoin/issues/11
Close https://github.com/project-loupe/audit-rust-bitcoin/issues/21
Close https://github.com/project-loupe/audit-rust-bitcoin/issues/127
Close https://github.com/project-loupe/audit-rust-bitcoin/issues/202
It was possible to feed massive alloc requests via the size hint of vec deserializers that comes directly from untrusted sources into units and primitives. The deserialized `SeqAccess::size_hint` value fed straight into `Vec::with_capacity`. A few bytes could request a huge allocation before anything was read.
This condition enables panics with a capacity overflow message or process abortions for excessive memory usage in downstream rust-bitcoin users using length prefixed formats (bincode, postcard, CBOR...).
This PR added a shared helper in `internals`, because of the universal applicability and to use both on units or primitives. I reviewed every hint size alloc and only found those primitives and units cases to be eligible for this fix.
Additionally, I found another instance on p2p, same class of bug, but it is out of scope because it is a consensus decoder, not serde.
ACKs for top commit:
apoelstra:
ACK f7c0ee459ed33fdade8977392f31bd8793e59d1a; successfully ran local tests
tcharding:
ACK f7c0ee459ed33fdade8977392f31bd8793e59d1a
Tree-SHA512: e11c71c7a3a12a627a528852cefe5d0469c55bd95fb370a11c688df921ad2af69c6f6b66371c68d756066994d8511673a3d966e6590254c72e104b2803636390
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit fixes a denial-of-service weakness in how the library deserializes lists of Bitcoin data (witnesses, amounts, fee rates) from untrusted input. Before the fix, a few bytes of attacker-controlled data could claim a list would contain billions of items, causing the program to reserve a huge block of memory and crash or be killed. The patch caps how much memory is reserved up front, matching a well-known safeguard used by the serde library itself.
Recommended action
Upgrade to a release containing this merge commit. If you maintain downstream code that deserializes rust-bitcoin types from length-prefixed formats (bincode, postcard, CBOR, etc.), ensure you are on the patched version. No other immediate action is required; the fix is self-contained and includes regression tests.
Security signals we found
01
Untrusted serde size hint fed directly into Vec::with_capacity
02
Potential memory exhaustion / OOM kill from small malicious input
03
Denial-of-service vector in deserialization paths
04
Fix mirrors serde's own 1 MiB preallocation cap
05
Regression tests simulate maximum possible size hint
Evidence from the diff
The merge introduces internals::serde::cautious_size_hint, which clamps SeqAccess::size_hint() to at most 1 MiB worth of elements (mirroring serde’s own Vec helper) before calling Vec::with_capacity. It replaces raw Vec::with_capacity(seq.size_hint().unwrap_or(0)) calls in primitives/src/witness.rs and several units serde modules (amount::serde::{as_sat,as_btc,as_str} and fee_rate::serde::{as_sat_per_kwu_floor,as_sat_per_vb_floor,as_sat_per_vb_ceil}). Regression tests use a fake SeqAccess returning usize::MAX to prove deserialization still succeeds with an empty result instead of attempting a massive allocation. The PR description notes a similar issue exists in p2p but was left out of scope because it is a consensus decoder, not serde.
Changed components
internals/src/serde.rsprimitives/src/witness.rsunits/src/amount/serde.rsunits/src/fee_rate/serde.rsInspect captured patch +88 / −10
### internals/src/serde.rs
@@ -65,3 +65,14 @@ macro_rules! serde_string_impl {
$crate::serde_string_serialize_impl!($name, $expecting);
};
}
+
+/// Upper and lower bound a sequence size hint before preallocating, the same way serde does for `Vec`.
+pub fn cautious_size_hint<T>(hint: Option<usize>) -> usize {
+ // The 1MB limit below was taken from serde
+ // https://github.com/serde-rs/serde/blob/master/serde_core/src/private/size_hint.rs
+ const MAX_PREALLOC_BYTES: usize = 1024 * 1024;
+ match MAX_PREALLOC_BYTES.checked_div(core::mem::size_of::<T>()) {
+ Some(max) => hint.unwrap_or(0).min(max),
+ None => 0,
+ }
+}
### primitives/src/witness.rs
@@ -811,10 +811,9 @@ impl<'de> serde::Deserialize<'de> for Witness {
self,
mut a: A,
) -> Result<Self::Value, A::Error> {
- let mut ret = match a.size_hint() {
- Some(len) => Vec::with_capacity(len),
- None => Vec::new(),
- };
+ let mut ret: Vec<Vec<u8>> = Vec::with_capacity(
+ internals::serde::cautious_size_hint::<Vec<u8>>(a.size_hint()),
+ );
while let Some(elem) = a.next_element::<String>()? {
let vec = hex::decode_to_vec(&elem).map_err(serde::de::Error::custom)?;
@@ -1384,6 +1383,29 @@ mod test {
witness
}
+ #[test]
+ #[cfg(feature = "serde")]
+ fn serde_does_not_trust_size_hint() {
+ use serde::de::value::{Error, SeqAccessDeserializer};
+ use serde::de::{DeserializeSeed, SeqAccess};
+
+ struct FakeHugeHint;
+ impl<'de> SeqAccess<'de> for FakeHugeHint {
+ type Error = Error;
+ fn next_element_seed<T: DeserializeSeed<'de>>(
+ &mut self,
+ _: T,
+ ) -> Result<Option<T::Value>, Error> {
+ Ok(None)
+ }
+ fn size_hint(&self) -> Option<usize> { Some(usize::MAX) }
+ }
+
+ let witness: Witness =
+ serde::Deserialize::deserialize(SeqAccessDeserializer::new(FakeHugeHint)).unwrap();
+ assert!(witness.is_empty());
+ }
+
#[test]
#[cfg(feature = "serde")]
fn serde_bincode_roundtrips() {
### units/src/amount/serde.rs
@@ -281,7 +281,9 @@ pub mod as_sat {
#[serde(transparent)]
struct Wrapper<T: TryFrom<SignedAmount>>(#[serde(with = "super")] T) where T::Error: core::fmt::Display;
- let mut out = Vec::with_capacity(seq.size_hint().unwrap_or(0));
+ let mut out = Vec::with_capacity(internals::serde::cautious_size_hint::<X>(
+ seq.size_hint(),
+ ));
while let Some(wrapped) = seq.next_element::<Wrapper<X>>()? {
out.push(wrapped.0);
}
@@ -449,7 +451,9 @@ pub mod as_btc {
#[serde(transparent)]
struct Wrapper(#[serde(with = "super")] SignedAmount);
- let mut out = Vec::with_capacity(seq.size_hint().unwrap_or(0));
+ let mut out = Vec::with_capacity(internals::serde::cautious_size_hint::<X>(
+ seq.size_hint(),
+ ));
while let Some(wrapped) = seq.next_element::<Wrapper>()? {
out.push(X::try_from(wrapped.0).map_err(de::Error::custom)?);
}
@@ -619,7 +623,9 @@ pub mod as_str {
#[serde(transparent)]
struct Wrapper(#[serde(with = "super")] SignedAmount);
- let mut out = Vec::with_capacity(seq.size_hint().unwrap_or(0));
+ let mut out = Vec::with_capacity(internals::serde::cautious_size_hint::<X>(
+ seq.size_hint(),
+ ));
while let Some(wrapped) = seq.next_element::<Wrapper>()? {
out.push(X::try_from(wrapped.0).map_err(de::Error::custom)?);
}
### units/src/fee_rate/serde.rs
@@ -139,7 +139,9 @@ pub mod as_sat_per_kwu_floor {
#[serde(transparent)]
struct Wrapper(#[serde(with = "super")] FeeRate);
- let mut out = Vec::with_capacity(seq.size_hint().unwrap_or(0));
+ let mut out = Vec::with_capacity(
+ internals::serde::cautious_size_hint::<FeeRate>(seq.size_hint()),
+ );
while let Some(wrapped) = seq.next_element::<Wrapper>()? {
out.push(wrapped.0);
}
@@ -265,7 +267,9 @@ pub mod as_sat_per_vb_floor {
#[serde(transparent)]
struct Wrapper(#[serde(with = "super")] FeeRate);
- let mut out = Vec::with_capacity(seq.size_hint().unwrap_or(0));
+ let mut out = Vec::with_capacity(
+ internals::serde::cautious_size_hint::<FeeRate>(seq.size_hint()),
+ );
while let Some(wrapped) = seq.next_element::<Wrapper>()? {
out.push(wrapped.0);
}
@@ -391,7 +395,9 @@ pub mod as_sat_per_vb_ceil {
#[serde(transparent)]
struct Wrapper(#[serde(with = "super")] FeeRate);
- let mut out = Vec::with_capacity(seq.size_hint().unwrap_or(0));
+ let mut out = Vec::with_capacity(
+ internals::serde::cautious_size_hint::<FeeRate>(seq.size_hint()),
+ );
while let Some(wrapped) = seq.next_element::<Wrapper>()? {
out.push(wrapped.0);
}
### units/tests/serde.rs
@@ -864,3 +864,36 @@ fn serde_regression_target() {
let want = include_bytes!("data/u256_bincode") as &[_];
assert_eq!(got, want);
}
+
+#[test]
+fn vec_deserializers_do_not_trust_size_hint() {
+ use serde::de::value::{Error, SeqAccessDeserializer};
+ use serde::de::{DeserializeSeed, SeqAccess};
+
+ struct FakeHugeHint;
+ impl<'de> SeqAccess<'de> for FakeHugeHint {
+ type Error = Error;
+ fn next_element_seed<T: DeserializeSeed<'de>>(
+ &mut self,
+ _: T,
+ ) -> Result<Option<T::Value>, Error> {
+ Ok(None)
+ }
+ fn size_hint(&self) -> Option<usize> { Some(usize::MAX) }
+ }
+
+ macro_rules! check_vec_alloc_succeeds {
+ ($($deserializer:path => $expected_type:ty),* $(,)?) => {$(
+ let got: Vec<$expected_type> = $deserializer(SeqAccessDeserializer::new(FakeHugeHint)).unwrap();
+ assert!(got.is_empty());
+ )*};
+ }
+ check_vec_alloc_succeeds!(
+ amount::serde::as_sat::vec::deserialize => Amount,
+ amount::serde::as_btc::vec::deserialize => Amount,
+ amount::serde::as_str::vec::deserialize => Amount,
+ fee_rate::serde::as_sat_per_kwu_floor::vec::deserialize => FeeRate,
+ fee_rate::serde::as_sat_per_vb_floor::vec::deserialize => FeeRate,
+ fee_rate::serde::as_sat_per_vb_ceil::vec::deserialize => FeeRate,
+ );
+}Risk score
Our methodology →Why this scored 70/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.