Skip to content

Commit

Permalink
Fix Array Deserialization Panic
Browse files Browse the repository at this point in the history
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
ggwpez committed Jun 19, 2024
1 parent 9ce37d5 commit ee3d528
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
27 changes: 22 additions & 5 deletions serialize/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use ark_std::{
collections::{BTreeMap, BTreeSet, LinkedList, VecDeque},
io::{Read, Write},
marker::PhantomData,
mem,
mem::MaybeUninit,
rc::Rc,
string::*,
vec::*,
Expand Down Expand Up @@ -458,13 +460,28 @@ impl<T: CanonicalDeserialize, const N: usize> CanonicalDeserialize for [T; N] {
compress: Compress,
validate: Validate,
) -> Result<Self, SerializationError> {
let result = core::array::from_fn(|_| {
T::deserialize_with_mode(&mut reader, compress, Validate::No).unwrap()
});
let mut array: [MaybeUninit<T>; N] = unsafe { MaybeUninit::uninit().assume_init() };

for i in 0..N {
// NOTE: If this `deserialize_with_mode` panics, then we have a memory leak.
array[i].write(T::deserialize_with_mode(
&mut reader,
compress,
Validate::No,
)?);
}
// SAFETY: All elements have been initialized and the size of `Dst` and `Src` are the same.
debug_assert_eq!(
mem::size_of::<[T; N]>(),
mem::size_of::<[MaybeUninit<T>; N]>()
);
let array: [T; N] = unsafe { mem::transmute_copy(&array) };

if let Validate::Yes = validate {
T::batch_check(result.iter())?
T::batch_check(array.iter())?
}
Ok(result)

Ok(array)
}
}

Expand Down
2 changes: 1 addition & 1 deletion serialize/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
rust_2018_idioms,
rust_2021_compatibility
)]
#![forbid(unsafe_code)]
// TODO #![forbid(unsafe_code)]
#![doc = include_str!("../README.md")]
mod error;
mod flags;
Expand Down
7 changes: 7 additions & 0 deletions serialize/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ fn test_array() {
test_serialize([1u8; 33]);
}

#[test]
fn test_array_bad_input() {
// Does not panic on invalid data:
let serialized = vec![0u8; 1];
assert!(<[u8; 2]>::deserialize_compressed(&serialized[..]).is_err());
}

#[test]
fn test_vec() {
test_serialize(vec![1u64, 2, 3, 4, 5]);
Expand Down

0 comments on commit ee3d528

Please sign in to comment.