chore: reject oversized bounded sequences early during deserialization (#3424)

core/src/sdp/mod.rs

@@ -626,11 +626,12 @@ mod tests {
     fn empty_locators_fail_to_deserialize() {
         let empty_locators = Vec::<Locator>::new();
         let serialized = serde_json::to_string(&empty_locators).unwrap();
-        assert_eq!(
+        assert!(
             serde_json::from_str::<Locators>(&serialized)
                 .unwrap_err()
-                .to_string(),
-            "Input cannot be empty."
+                .to_string()
+                .contains("Input cannot be empty."),
+            "empty locators should be rejected"
         );
     }
 

mmr/src/path.rs

@@ -113,11 +113,8 @@ pub enum MerklePathError {
 
 mod serde_siblings {
     use lb_groth16::{Fr, serde::serde_fr_vec};
-    use lb_utils::bounded::UpperBoundedVec;
-    use serde::{
-        Deserialize, Deserializer, Serializer,
-        de::{SeqAccess, Visitor},
-    };
+    use lb_utils::bounded::{UpperBoundedVec, deserialize_bounded_sequence};
+    use serde::{Deserialize, Deserializer, Serializer};
 
     use super::{MAX_MERKLE_PATH_SIBLINGS, MerklePathSiblings};
 
@@ -135,35 +132,15 @@ mod serde_siblings {
         #[derive(Deserialize)]
         struct FrWrap(#[serde(with = "lb_groth16::serde::serde_fr")] Fr);
 
-        struct SiblingsVisitor;
-
-        impl<'de> Visitor<'de> for SiblingsVisitor {
-            type Value = Vec<Fr>;
-
-            fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-                formatter.write_str("a sequence of MMR sibling field elements")
-            }
-
-            fn visit_seq<A>(self, mut sequence: A) -> Result<Self::Value, A::Error>
-            where
-                A: SeqAccess<'de>,
-            {
-                let mut siblings = Vec::with_capacity(MAX_MERKLE_PATH_SIBLINGS);
-                while let Some(FrWrap(sibling)) = sequence.next_element()? {
-                    if siblings.len() == MAX_MERKLE_PATH_SIBLINGS {
-                        return Err(serde::de::Error::custom(format_args!(
-                            "MMR path contains more than {MAX_MERKLE_PATH_SIBLINGS} siblings"
-                        )));
-                    }
-                    siblings.push(sibling);
-                }
-                Ok(siblings)
-            }
-        }
-
-        let siblings = deserializer.deserialize_seq(SiblingsVisitor)?;
-        UpperBoundedVec::<Fr, MAX_MERKLE_PATH_SIBLINGS>::try_from(siblings)
-            .map_err(serde::de::Error::custom)
+        let siblings =
+            deserialize_bounded_sequence::<FrWrap, 0, MAX_MERKLE_PATH_SIBLINGS, D>(deserializer)?;
+        Ok(UpperBoundedVec::new_unchecked(
+            siblings
+                .into_inner()
+                .into_iter()
+                .map(|FrWrap(sibling)| sibling)
+                .collect(),
+        ))
     }
 }
 

utils/src/bounded/mod.rs

@@ -2,8 +2,8 @@
 //! inclusive `[MIN, MAX]` range.
 //!
 //! [`Bounded`] captures the machinery shared by every length-bounded type in
-//! the codebase — bound checking, unchecked/checked construction, transparent
-//! serialization and validating deserialization — so that concrete bounded
+//! the codebase — bound checking, unchecked/checked construction, and
+//! transparent serialization — so that concrete bounded
 //! types (`BoundedVec`, chain IDs, locators, …) reduce to a type alias plus
 //! whatever operations are natural for the wrapped type.
 //!
@@ -19,10 +19,13 @@ pub mod vec;
 
 use core::fmt::{self, Display, Formatter};
 
-use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Serialize, Serializer};
 pub use string::BoundedString;
 use thiserror::Error;
-pub use vec::{BoundedVec, LowerBoundedVec, MaxBoundedVec, NonEmptyBoundedVec, UpperBoundedVec};
+pub use vec::{
+    BoundedVec, LowerBoundedVec, MaxBoundedVec, NonEmptyBoundedVec, UpperBoundedVec,
+    deserialize_bounded_sequence,
+};
 
 #[derive(Debug, Error, Eq, PartialEq, Clone)]
 pub enum BoundedError {
@@ -46,7 +49,7 @@ pub enum BoundedError {
 /// element count for collections, byte length for text.
 ///
 /// Implementing this for a type unlocks the ergonomic checked constructors on
-/// [`Bounded`] ([`Bounded::new`], `TryFrom`, `Deserialize`, [`Bounded::len`]).
+/// [`Bounded`] ([`Bounded::new`], `TryFrom`, [`Bounded::len`]).
 /// Foreign types that cannot get an impl here can still be bounded manually via
 /// [`Bounded::check_len`] + [`Bounded::new_unchecked`].
 pub trait BoundedLen {
@@ -152,14 +155,3 @@ where
         self.0.serialize(serializer)
     }
 }
-
-// Deserialize the inner `T`, then re-establish the bound before wrapping.
-impl<'de, T, const MIN: usize, const MAX: usize> Deserialize<'de> for Bounded<T, MIN, MAX>
-where
-    T: BoundedLen + Deserialize<'de>,
-{
-    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-        let inner = T::deserialize(deserializer)?;
-        Self::try_new(inner).map_err(serde::de::Error::custom)
-    }
-}

utils/src/bounded/multiaddr.rs

@@ -1,4 +1,5 @@
 use multiaddr::Multiaddr;
+use serde::{Deserialize, Deserializer};
 
 use crate::bounded::{Bounded, BoundedError, BoundedLen, BoundedVec};
 
@@ -11,11 +12,19 @@ impl BoundedLen for Multiaddr {
 /// A `Multiaddr` whose byte length is statically enforced to be in the range
 /// `[MIN, MAX]`.
 ///
-/// A thin alias over [`Bounded`]. Length checking, (de)serialization, `Display`
-/// and unchecked construction all come from the generic wrapper; only the
-/// multiaddr-flavoured conversions live here.
+/// A thin alias over [`Bounded`]. Length checking, serialization, `Display` and
+/// unchecked construction come from the generic wrapper; multiaddr
+/// deserialization and the remaining multiaddr-flavoured conversions live
+/// here.
 pub type BoundedMultiaddr<const MIN: usize, const MAX: usize> = Bounded<Multiaddr, MIN, MAX>;
 
+impl<'de, const MIN: usize, const MAX: usize> Deserialize<'de> for BoundedMultiaddr<MIN, MAX> {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let value = Multiaddr::deserialize(deserializer)?;
+        Self::try_new(value).map_err(serde::de::Error::custom)
+    }
+}
+
 impl<const MIN: usize, const MAX: usize> BoundedMultiaddr<MIN, MAX> {
     /// Length in bytes (not `char`s), matching `Multiaddr` semantics.
     #[must_use]

utils/src/bounded/string.rs

@@ -1,3 +1,5 @@
+use serde::{Deserialize, Deserializer};
+
 use crate::bounded::{Bounded, BoundedError, BoundedLen, BoundedVec};
 
 impl BoundedLen for String {
@@ -9,11 +11,18 @@ impl BoundedLen for String {
 /// A `String` whose byte length is statically enforced to be in the range
 /// `[MIN, MAX]`.
 ///
-/// A thin alias over [`Bounded`]. Length checking, (de)serialization, `Display`
-/// and unchecked construction all come from the generic wrapper; only the
-/// string-flavoured conversions live here.
+/// A thin alias over [`Bounded`]. Length checking, serialization, `Display` and
+/// unchecked construction come from the generic wrapper; string deserialization
+/// and the remaining string-flavoured conversions live here.
 pub type BoundedString<const MIN: usize, const MAX: usize> = Bounded<String, MIN, MAX>;
 
+impl<'de, const MIN: usize, const MAX: usize> Deserialize<'de> for BoundedString<MIN, MAX> {
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        let value = String::deserialize(deserializer)?;
+        Self::try_new(value).map_err(serde::de::Error::custom)
+    }
+}
+
 impl<const MIN: usize, const MAX: usize> BoundedString<MIN, MAX> {
     /// Length in bytes (not `char`s), matching `str`/`String` semantics.
     #[must_use]

utils/src/bounded/vec.rs

@@ -1,9 +1,15 @@
 use core::{
+    marker::PhantomData,
     ops::Deref,
     slice::{Iter, IterMut},
 };
 use std::{ops::DerefMut, str::FromStr, vec::IntoIter};
 
+use serde::{
+    Deserialize, Deserializer,
+    de::{Error as _, SeqAccess, Visitor},
+};
+
 use crate::bounded::{Bounded, BoundedError, BoundedLen};
 
 impl<T> BoundedLen for Vec<T> {
@@ -15,15 +21,85 @@ impl<T> BoundedLen for Vec<T> {
 /// `Vec<T>` whose length is statically enforced to be in the range `[MIN,
 /// MAX]`.
 ///
-/// A thin alias over [`Bounded`]: the length checking, (de)serialization and
-/// construction machinery lives on the generic wrapper, while the operations
-/// below are the ones that only make sense for a `Vec`.
+/// A thin alias over [`Bounded`]: the length checking and construction
+/// machinery lives on the generic wrapper, while sequence deserialization and
+/// the operations below are the ones that only make sense for a `Vec`.
 ///
 /// The invariant is enforced at every checked construction site
 /// ([`TryFrom<Vec<T>>`](Self::try_from), deserialization), so an instance can
 /// never be shorter than `MIN` nor longer than `MAX`.
 pub type BoundedVec<T, const MIN: usize, const MAX: usize> = Bounded<Vec<T>, MIN, MAX>;
 
+impl<'de, T, const MIN: usize, const MAX: usize> Deserialize<'de> for Bounded<Vec<T>, MIN, MAX>
+where
+    T: Deserialize<'de>,
+{
+    fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+        deserialize_bounded_sequence(deserializer)
+    }
+}
+
+/// Deserialize a sequence directly into a bounded vector.
+///
+/// Sequence formats may provide a length through [`SeqAccess::size_hint`].
+/// When that length exceeds `MAX`, it is rejected before any element is
+/// decoded. Formats without a reliable hint are still bounded by stopping at
+/// the first element beyond `MAX`.
+pub fn deserialize_bounded_sequence<'de, T, const MIN: usize, const MAX: usize, D>(
+    deserializer: D,
+) -> Result<BoundedVec<T, MIN, MAX>, D::Error>
+where
+    T: Deserialize<'de>,
+    D: Deserializer<'de>,
+{
+    deserializer.deserialize_seq(BoundedSequenceVisitor {
+        marker: PhantomData,
+    })
+}
+
+struct BoundedSequenceVisitor<T, const MIN: usize, const MAX: usize> {
+    marker: PhantomData<T>,
+}
+
+impl<'de, T, const MIN: usize, const MAX: usize> Visitor<'de>
+    for BoundedSequenceVisitor<T, MIN, MAX>
+where
+    T: Deserialize<'de>,
+{
+    type Value = BoundedVec<T, MIN, MAX>;
+
+    fn expecting(&self, formatter: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(formatter, "a sequence with between {MIN} and {MAX} items")
+    }
+
+    fn visit_seq<A>(self, mut sequence: A) -> Result<Self::Value, A::Error>
+    where
+        A: SeqAccess<'de>,
+    {
+        let size_hint = sequence.size_hint();
+        if let Some(size_hint) = size_hint.filter(|&size_hint| size_hint > MAX) {
+            return Err(A::Error::custom(BoundedError::TooManyItems {
+                count: size_hint,
+                max: MAX,
+            }));
+        }
+
+        let capacity = size_hint.unwrap_or(0).min(MAX);
+        let mut values = Vec::with_capacity(capacity);
+        while let Some(value) = sequence.next_element()? {
+            if values.len() == MAX {
+                return Err(A::Error::custom(BoundedError::TooManyItems {
+                    count: MAX.saturating_add(1),
+                    max: MAX,
+                }));
+            }
+            values.push(value);
+        }
+
+        BoundedVec::try_from(values).map_err(A::Error::custom)
+    }
+}
+
 impl<T, const MIN: usize, const MAX: usize> Bounded<Vec<T>, MIN, MAX> {
     /// Constructs an empty vector.
     ///
@@ -343,6 +419,13 @@ pub type MaxBoundedVec<T> = UpperBoundedVec<T, { usize::MAX }>;
 
 #[cfg(test)]
 mod tests {
+    use std::sync::{
+        Mutex,
+        atomic::{AtomicUsize, Ordering},
+    };
+
+    use serde::{Deserialize, Deserializer};
+
     use crate::bounded::{BoundedError, BoundedVec, UpperBoundedVec};
 
     /// Concrete instantiation used across the tests: between 2 and 4 elements.
@@ -350,6 +433,18 @@ mod tests {
     type TestBoundedVectorMin1 = BoundedVec<u8, 1, 4>;
     type TestBoundedVectorMin0 = BoundedVec<u8, 0, 4>;
 
+    static ELEMENT_ATTEMPTS: AtomicUsize = AtomicUsize::new(0);
+    static ELEMENT_ATTEMPTS_TEST_LOCK: Mutex<()> = Mutex::new(());
+
+    struct CountingByte;
+
+    impl<'de> Deserialize<'de> for CountingByte {
+        fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
+            ELEMENT_ATTEMPTS.fetch_add(1, Ordering::Relaxed);
+            u8::deserialize(deserializer).map(|_| Self)
+        }
+    }
+
     #[test]
     fn from_accepts_single_element_construction() {
         let single = TestBoundedVectorMin0::from(1);
@@ -546,6 +641,15 @@ mod tests {
         assert_eq!(bv.as_slice(), &[1, 2, 3]);
     }
 
+    #[test]
+    fn deserialize_accepts_inputs_at_bounds() {
+        let min: TestBoundedVectorMin2 = serde_json::from_str("[1,2]").unwrap();
+        assert_eq!(min.as_slice(), &[1, 2]);
+
+        let max: TestBoundedVectorMin2 = serde_json::from_str("[1,2,3,4]").unwrap();
+        assert_eq!(max.as_slice(), &[1, 2, 3, 4]);
+    }
+
     #[test]
     fn serialize_then_deserialize_roundtrips() {
         let original = TestBoundedVectorMin2::try_from(vec![5, 6, 7, 8]).unwrap();
@@ -582,6 +686,39 @@ mod tests {
         );
     }
 
+    #[test]
+    fn deserialize_json_stops_after_at_most_one_element_past_maximum() {
+        let _test_guard = ELEMENT_ATTEMPTS_TEST_LOCK.lock().unwrap();
+        ELEMENT_ATTEMPTS.store(0, Ordering::Relaxed);
+
+        let result = serde_json::from_str::<BoundedVec<CountingByte, 0, 4>>("[1,2,3,4,5,6]");
+
+        assert!(result.is_err());
+        assert!(ELEMENT_ATTEMPTS.load(Ordering::Relaxed) <= 5);
+    }
+
+    #[test]
+    fn deserialize_binary_rejects_oversized_length_before_decoding_elements() {
+        let _test_guard = ELEMENT_ATTEMPTS_TEST_LOCK.lock().unwrap();
+        ELEMENT_ATTEMPTS.store(0, Ordering::Relaxed);
+        let encoded = bincode::serialize(&vec![1u8; 5]).unwrap();
+
+        let result = bincode::deserialize::<BoundedVec<CountingByte, 0, 4>>(&encoded);
+
+        assert!(result.is_err());
+        assert_eq!(ELEMENT_ATTEMPTS.load(Ordering::Relaxed), 0);
+    }
+
+    #[test]
+    fn deserialize_binary_preserves_the_vector_wire_format() {
+        let original = TestBoundedVectorMin2::try_from(vec![5, 6, 7]).unwrap();
+        let encoded = bincode::serialize(&original).unwrap();
+        let restored = bincode::deserialize::<TestBoundedVectorMin2>(&encoded).unwrap();
+
+        assert_eq!(restored, original);
+        assert_eq!(encoded, bincode::serialize(&vec![5u8, 6, 7]).unwrap());
+    }
+
     #[test]
     fn try_pop_returns_none_at_or_below_lower_bound_and_is_idempotent() {
         let mut bv = TestBoundedVectorMin2::try_from(vec![1, 2, 3]).unwrap();