From 12169619b82b9cae6dcd226444d213e2cef900c7 Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Wed, 23 Aug 2023 14:17:37 +0000 Subject: [PATCH 1/2] Fix value explosion in arbitrary fuzzer --- fuzz/fuzz_targets/bench/lib.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/fuzz/fuzz_targets/bench/lib.rs b/fuzz/fuzz_targets/bench/lib.rs index 9176cf30..84500ced 100644 --- a/fuzz/fuzz_targets/bench/lib.rs +++ b/fuzz/fuzz_targets/bench/lib.rs @@ -30,6 +30,7 @@ pub fn roundtrip_arbitrary_typed_ron_or_panic(data: &[u8]) -> Option panic!("{:?} -! {:?}", typed_value, err), }; + if let Err(err) = ron::Options::default().from_str::(&ron) { match err.code { // Erroring on deep recursion is better than crashing on a stack overflow @@ -38,6 +39,7 @@ pub fn roundtrip_arbitrary_typed_ron_or_panic(data: &[u8]) -> Option panic!("{:?} -> {} -! {:?}", typed_value, ron, err), } }; + if let Err(err) = ron::Options::default().from_str_seed(&ron, &typed_value) { match err.code { // Erroring on deep recursion is better than crashing on a stack overflow @@ -46,7 +48,7 @@ pub fn roundtrip_arbitrary_typed_ron_or_panic(data: &[u8]) -> Option panic!("{:?} -> {} -! {:?}", typed_value, ron, err), } }; - // TODO: also do typed deserialise + Some(typed_value) } else { None @@ -1422,19 +1424,23 @@ impl<'a> SerdeDataType<'a> { Ok(SerdeDataValue::Seq { elems: tuple }) } SerdeDataType::Vec { item } => { - let len = u.arbitrary_len::()?.min(4); - let mut vec = Vec::with_capacity(len); - for _ in 0..len { + let mut vec = Vec::new(); + vec.push(item.arbitrary_value(u)?); + + while u.arbitrary()? { vec.push(item.arbitrary_value(u)?); } + Ok(SerdeDataValue::Seq { elems: vec }) } SerdeDataType::Map { key, value } => { - let len = u.arbitrary_len::()?.min(4); - let mut map = Vec::with_capacity(len); - for _ in 0..len { + let mut map = Vec::new(); + map.push((key.arbitrary_value(u)?, value.arbitrary_value(u)?)); + + while u.arbitrary()? { map.push((key.arbitrary_value(u)?, value.arbitrary_value(u)?)); } + Ok(SerdeDataValue::Map { elems: map }) } SerdeDataType::UnitStruct { name: _ } => Ok(SerdeDataValue::UnitStruct), @@ -1567,11 +1573,13 @@ fn arbitrary_str_tuple_vec_recursion_guard<'a, T: Arbitrary<'a>>( .map_or(256, |limit| limit * 2); let result = if RECURSION_DEPTH.fetch_add(1, Ordering::Relaxed) < max_depth { - let len = u.arbitrary_len::<(&str, T)>()?; - let mut s = Vec::with_capacity(len); - let mut v = Vec::with_capacity(len); + let mut s = Vec::new(); + let mut v = Vec::new(); + + s.push(<&str>::arbitrary(u)?); + v.push(T::arbitrary(u)?); - for _ in 0..len { + while u.arbitrary()? { s.push(<&str>::arbitrary(u)?); v.push(T::arbitrary(u)?); } From cbf7fd6aae936e05b75d47a8fbc5ab0746d7713f Mon Sep 17 00:00:00 2001 From: Juniper Tyree <50025784+juntyr@users.noreply.github.com> Date: Wed, 23 Aug 2023 16:19:52 +0000 Subject: [PATCH 2/2] Improve the fuzzing of arrays with a max spread --- fuzz/fuzz_targets/bench/lib.rs | 77 +++++++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 20 deletions(-) diff --git a/fuzz/fuzz_targets/bench/lib.rs b/fuzz/fuzz_targets/bench/lib.rs index 84500ced..4d9218e0 100644 --- a/fuzz/fuzz_targets/bench/lib.rs +++ b/fuzz/fuzz_targets/bench/lib.rs @@ -191,7 +191,12 @@ impl<'a> Serialize for BorrowedTypedSerdeData<'a> { serializer.serialize_none() } } - (SerdeDataType::Array { kind, len }, SerdeDataValue::Seq { elems }) => { + ( + SerdeDataType::Array { + kind_len: (kind, len), + }, + SerdeDataValue::Seq { elems }, + ) => { if elems.len() != *len { return Err(serde::ser::Error::custom("mismatch array len")); } @@ -532,7 +537,12 @@ impl<'a, 'de> DeserializeSeed<'de> for BorrowedTypedSerdeData<'a> { value: value.as_deref(), }) } - (SerdeDataType::Array { kind, len }, SerdeDataValue::Seq { elems }) => { + ( + SerdeDataType::Array { + kind_len: (kind, len), + }, + SerdeDataValue::Seq { elems }, + ) => { struct ArrayVisitor<'a> { kind: &'a SerdeDataType<'a>, elems: &'a [SerdeDataValue<'a>], @@ -1329,10 +1339,8 @@ pub enum SerdeDataType<'a> { inner: Box, }, Array { - #[arbitrary(with = arbitrary_recursion_guard)] - kind: Box, - #[arbitrary(with = arbitrary_recursion_guard)] - len: usize, + #[arbitrary(with = arbitrary_array_len_recursion_guard)] + kind_len: (Box, usize), }, Tuple { #[arbitrary(with = arbitrary_recursion_guard)] @@ -1405,11 +1413,9 @@ impl<'a> SerdeDataType<'a> { }; Ok(SerdeDataValue::Option { inner: value }) } - SerdeDataType::Array { kind, len } => { - if *len > 32 { - // Restrict array lengths to be reasonable, as arbitrary cannot - return Err(arbitrary::Error::IncorrectFormat); - } + SerdeDataType::Array { + kind_len: (kind, len), + } => { let mut array = Vec::with_capacity(*len); for _ in 0..*len { array.push(kind.arbitrary_value(u)?); @@ -1425,22 +1431,16 @@ impl<'a> SerdeDataType<'a> { } SerdeDataType::Vec { item } => { let mut vec = Vec::new(); - vec.push(item.arbitrary_value(u)?); - while u.arbitrary()? { vec.push(item.arbitrary_value(u)?); } - Ok(SerdeDataValue::Seq { elems: vec }) } SerdeDataType::Map { key, value } => { let mut map = Vec::new(); - map.push((key.arbitrary_value(u)?, value.arbitrary_value(u)?)); - while u.arbitrary()? { map.push((key.arbitrary_value(u)?, value.arbitrary_value(u)?)); } - Ok(SerdeDataValue::Map { elems: map }) } SerdeDataType::UnitStruct { name: _ } => Ok(SerdeDataValue::UnitStruct), @@ -1565,6 +1565,46 @@ fn arbitrary_recursion_guard<'a, T: Arbitrary<'a> + Default>( result } +fn arbitrary_array_len_recursion_guard<'a>( + u: &mut Unstructured<'a>, +) -> arbitrary::Result<(Box>, usize)> { + const MAX_ARRAY_SPREAD: usize = 32; + static ARRAY_SPREAD: AtomicUsize = AtomicUsize::new(1); + + let max_depth = ron::Options::default() + .recursion_limit + .map_or(256, |limit| limit * 2); + + // Fall back to zero-sized arrays if the input is exhausted + if u.is_empty() { + return Ok((Box::new(SerdeDataType::Unit), 0)); + } + + let result = if RECURSION_DEPTH.fetch_add(1, Ordering::Relaxed) < max_depth { + // Limit the maximum array spread, since arbitrary cannot + let spread = ARRAY_SPREAD.load(Ordering::Relaxed); + let len = u.int_in_range::(0..=(MAX_ARRAY_SPREAD / spread).clamp(1, 16))?; + + // Only generate intricate inner array types if the length is non-zero + let kind = if len > 0 { + ARRAY_SPREAD.swap(spread * len, Ordering::Relaxed); + let kind = SerdeDataType::arbitrary(u)?; + ARRAY_SPREAD.swap(spread, Ordering::Relaxed); + kind + } else { + SerdeDataType::Unit + }; + + Ok((Box::new(kind), len)) + } else { + Ok((Box::new(SerdeDataType::Unit), 0)) + }; + + RECURSION_DEPTH.fetch_sub(1, Ordering::Relaxed); + + result +} + fn arbitrary_str_tuple_vec_recursion_guard<'a, T: Arbitrary<'a>>( u: &mut Unstructured<'a>, ) -> arbitrary::Result<(Vec<&'a str>, Vec)> { @@ -1576,9 +1616,6 @@ fn arbitrary_str_tuple_vec_recursion_guard<'a, T: Arbitrary<'a>>( let mut s = Vec::new(); let mut v = Vec::new(); - s.push(<&str>::arbitrary(u)?); - v.push(T::arbitrary(u)?); - while u.arbitrary()? { s.push(<&str>::arbitrary(u)?); v.push(T::arbitrary(u)?);