From 733886a166c0b38b48dfa5688d71683721afe3a4 Mon Sep 17 00:00:00 2001 From: scadastrangelove Date: Mon, 20 Jul 2026 11:10:08 +0300 Subject: [PATCH] Extend depth-limit guard to deserialize_option, deserialize_newtype_struct, and newtype_variant_seed depth_count! (added in #277) only wrapped visit_seq/visit_map/the Ext-marker visit_newtype_struct. This extends it to the three remaining recursion-capable forwarding points in Deserializer/VariantAccess: - deserialize_option - deserialize_newtype_struct (both branches) - VariantAccess::newtype_variant_seed Each site needs an explicit &mut *self/&mut *de reborrow rather than passing self/de directly, since a generic D: Deserializer<'de> parameter does not get an implicit reborrow the way a concretely-typed &mut T parameter does. Adds fail_depth_limit_option, fail_depth_limit_newtype_struct, and fail_depth_limit_newtype_variant regression tests, mirroring the existing fail_depth_limit test. Discovered by the rust-in-peace security pipeline (https://github.com/scadastrangelove/rust-in-peace/). --- rmp-serde/src/decode.rs | 10 ++-- rmp-serde/tests/decode.rs | 123 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/rmp-serde/src/decode.rs b/rmp-serde/src/decode.rs index 2c71e3bc..5a6d0fc8 100644 --- a/rmp-serde/src/decode.rs +++ b/rmp-serde/src/decode.rs @@ -658,7 +658,7 @@ impl<'de, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> for & } else { // Keep the marker until `o`'s innermost type `t` is visited. self.marker = Some(marker); - visitor.visit_some(self) + depth_count!(self.depth, visitor.visit_some(&mut *self)) } } @@ -689,11 +689,10 @@ impl<'de, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> for & let marker = self.take_or_read_marker()?; let len = ext_len(&mut self.rd, marker)?; - let ext_de = ExtDeserializer::new(self, len); - return visitor.visit_newtype_struct(ext_de); + return depth_count!(self.depth, visitor.visit_newtype_struct(ExtDeserializer::new(self, len))); } - visitor.visit_newtype_struct(self) + depth_count!(self.depth, visitor.visit_newtype_struct(&mut *self)) } fn deserialize_unit_struct(self, _name: &'static str, visitor: V) -> Result @@ -987,7 +986,8 @@ impl<'de, R: ReadSlice<'de>, C: SerializerConfig> de::VariantAccess<'de> for Var fn newtype_variant_seed(self, seed: T) -> Result where T: DeserializeSeed<'de> { - seed.deserialize(self.de) + let de = self.de; + depth_count!(de.depth, seed.deserialize(&mut *de)) } #[inline] diff --git a/rmp-serde/tests/decode.rs b/rmp-serde/tests/decode.rs index 4763eb90..a8ff8fd5 100644 --- a/rmp-serde/tests/decode.rs +++ b/rmp-serde/tests/decode.rs @@ -567,3 +567,126 @@ fn fail_depth_limit() { other => panic!("unexpected result: {other:?}"), } } + +#[test] +fn fail_depth_limit_option() { + // `deserialize_option` forwarded to `visit_some` uncounted. Any non-nil marker (not just a + // deeply-nested one) drives unbounded recursion for an `Option>`-shaped type, + // because the same cached marker byte is reinterpreted at every level without being consumed. + #[allow(dead_code)] + struct Nested(Option>); + + impl<'de> de::Deserialize<'de> for Nested { + fn deserialize(de: D) -> Result + where D: de::Deserializer<'de> + { + Ok(Self(Option::deserialize(de)?)) + } + } + + // A single non-nil byte (a fixint 0, not the 0xc0 nil marker). + let data = vec![0x00u8]; + let mut reader = rmp_serde::Deserializer::new(Cursor::new(data)); + reader.set_max_depth(100); + let res = Nested::deserialize(&mut reader); + match res.err().unwrap() { + decode::Error::DepthLimitExceeded => (), + other => panic!("unexpected result: {other:?}"), + } +} + +#[test] +fn fail_depth_limit_newtype_struct() { + // `deserialize_newtype_struct`'s general (non-Ext) branch forwarded to + // `visit_newtype_struct` uncounted. For a newtype struct wrapping itself, this recurses + // without ever reading a byte -- even an EMPTY buffer drives unbounded recursion. + #[allow(dead_code)] + struct Wrap(Box); + + impl<'de> de::Deserialize<'de> for Wrap { + fn deserialize(de: D) -> Result + where D: de::Deserializer<'de> + { + de.deserialize_newtype_struct("Wrap", WrapVisitor) + } + } + + struct WrapVisitor; + impl<'de> de::Visitor<'de> for WrapVisitor { + type Value = Wrap; + + fn expecting(&self, f: &mut Formatter) -> fmt::Result { + f.write_str("a Wrap") + } + + fn visit_newtype_struct(self, de: D) -> Result + where D: de::Deserializer<'de> + { + Ok(Wrap(Box::new(Wrap::deserialize(de)?))) + } + } + + let data: &[u8] = &[]; + let mut reader = rmp_serde::Deserializer::new(data); + reader.set_max_depth(100); + let res = Wrap::deserialize(&mut reader); + match res.err().unwrap() { + decode::Error::DepthLimitExceeded => (), + other => panic!("unexpected result: {other:?}"), + } +} + +#[test] +fn fail_depth_limit_newtype_variant() { + // `VariantAccess::newtype_variant_seed` (the map-as-single-variant-enum path) forwarded to + // `seed.deserialize` uncounted. A chain of `n` `{"Node": ...}` maps around a `"Leaf"` string + // drives `n` levels of recursion, one `depth_count!` tick apiece once fixed. + #[allow(dead_code)] + enum EList { + Leaf, + Node(Box), + } + + impl<'de> de::Deserialize<'de> for EList { + fn deserialize(de: D) -> Result + where D: de::Deserializer<'de> + { + de.deserialize_enum("EList", &["Leaf", "Node"], EListVisitor) + } + } + + struct EListVisitor; + impl<'de> de::Visitor<'de> for EListVisitor { + type Value = EList; + + fn expecting(&self, f: &mut Formatter) -> fmt::Result { + f.write_str("an EList") + } + + fn visit_enum(self, data: A) -> Result + where A: de::EnumAccess<'de> + { + use de::VariantAccess; + let (tag, variant): (String, _) = data.variant()?; + if tag == "Node" { + Ok(EList::Node(Box::new(variant.newtype_variant()?))) + } else { + variant.unit_variant()?; + Ok(EList::Leaf) + } + } + } + + let mut data = Vec::new(); + for _ in 0..200 { + data.extend_from_slice(&[0x81, 0xa4, b'N', b'o', b'd', b'e']); + } + data.extend_from_slice(&[0xa4, b'L', b'e', b'a', b'f']); + let mut reader = rmp_serde::Deserializer::new(Cursor::new(data)); + reader.set_max_depth(100); + let res = EList::deserialize(&mut reader); + match res.err().unwrap() { + decode::Error::DepthLimitExceeded => (), + other => panic!("unexpected result: {other:?}"), + } +}