From 7daeb1f9df0ee564f13eb70d34f2b296d04c2c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 31 Mar 2026 14:37:59 -0300 Subject: [PATCH 1/2] hardening: add safety comments to unsafe blocks --- crates/ssz/src/decode.rs | 5 +++++ crates/ssz/src/encode.rs | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/crates/ssz/src/decode.rs b/crates/ssz/src/decode.rs index 6986f4d..b195377 100644 --- a/crates/ssz/src/decode.rs +++ b/crates/ssz/src/decode.rs @@ -262,6 +262,11 @@ fn decode_byte_array_vec(bytes: &[u8]) -> Result, D } let count = bytes.len() / N; let mut result = Vec::<[u8; N]>::with_capacity(count); + // SAFETY: `[u8; N]` has size `N`, alignment 1, no padding, and is valid + // for any bit pattern. The length check guarantees `count * N == + // bytes.len()`, and `with_capacity(count)` allocates at least `count * N` + // bytes, so the copy stays in bounds. No endianness concern since the + // element type is raw bytes. unsafe { core::ptr::copy_nonoverlapping(bytes.as_ptr(), result.as_mut_ptr() as *mut u8, bytes.len()); result.set_len(count); diff --git a/crates/ssz/src/encode.rs b/crates/ssz/src/encode.rs index 6feea40..ee6dd38 100644 --- a/crates/ssz/src/encode.rs +++ b/crates/ssz/src/encode.rs @@ -45,6 +45,12 @@ pub trait SszEncode { #[cfg(feature = "alloc")] fn append_fixed_slice_as_bytes(items: &[T], buf: &mut Vec) { + // SAFETY: Callers must only invoke this with types `T` that have no + // padding bytes and a valid representation for any bit pattern. Integer + // callers (u8..u128) are additionally cfg-guarded to little-endian + // platforms so the memory layout matches SSZ wire format. The [u8; N] + // caller is endianness-independent. Current callers: u8, u16, u32, u64, + // u128, [u8; N]. let byte_slice = unsafe { core::slice::from_raw_parts(items.as_ptr().cast::(), core::mem::size_of_val(items)) }; From 7cb81ad259627adf11d042bd76260678ed3368b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Tue, 31 Mar 2026 18:16:23 -0300 Subject: [PATCH 2/2] docs: refine safety comments based on review decode_byte_array_vec: drop "no padding" (not a soundness requirement for the decode direction; [u8; N] is valid for any bit pattern). append_fixed_slice_as_bytes: keep "no padding" (the encode direction reads T as bytes, and the Reference allows padding in valid T values to hold uninitialized bytes, making the u8 reinterpretation UB). --- crates/ssz/src/decode.rs | 8 +++----- crates/ssz/src/encode.rs | 44 +++++++++++++++++++++++++--------------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/crates/ssz/src/decode.rs b/crates/ssz/src/decode.rs index b195377..8c5070f 100644 --- a/crates/ssz/src/decode.rs +++ b/crates/ssz/src/decode.rs @@ -262,11 +262,9 @@ fn decode_byte_array_vec(bytes: &[u8]) -> Result, D } let count = bytes.len() / N; let mut result = Vec::<[u8; N]>::with_capacity(count); - // SAFETY: `[u8; N]` has size `N`, alignment 1, no padding, and is valid - // for any bit pattern. The length check guarantees `count * N == - // bytes.len()`, and `with_capacity(count)` allocates at least `count * N` - // bytes, so the copy stays in bounds. No endianness concern since the - // element type is raw bytes. + // SAFETY: The length check guarantees `count * N == bytes.len()`, and + // `with_capacity(count)` allocates at least `count * N` bytes, so the + // copy and `set_len` stay in bounds. unsafe { core::ptr::copy_nonoverlapping(bytes.as_ptr(), result.as_mut_ptr() as *mut u8, bytes.len()); result.set_len(count); diff --git a/crates/ssz/src/encode.rs b/crates/ssz/src/encode.rs index ee6dd38..0fdb917 100644 --- a/crates/ssz/src/encode.rs +++ b/crates/ssz/src/encode.rs @@ -43,17 +43,23 @@ pub trait SszEncode { } } +/// Reinterpret `&[T]` as `&[u8]` and append to `buf`. +/// +/// # Safety +/// +/// `T` must not have padding bytes. +/// +/// # Correctness +/// +/// `T` must have a little-endian in-memory layout matching SSZ wire +/// format, unless `T` is endianness-independent (e.g. `[u8; N]`). #[cfg(feature = "alloc")] -fn append_fixed_slice_as_bytes(items: &[T], buf: &mut Vec) { - // SAFETY: Callers must only invoke this with types `T` that have no - // padding bytes and a valid representation for any bit pattern. Integer - // callers (u8..u128) are additionally cfg-guarded to little-endian - // platforms so the memory layout matches SSZ wire format. The [u8; N] - // caller is endianness-independent. Current callers: u8, u16, u32, u64, - // u128, [u8; N]. - let byte_slice = unsafe { - core::slice::from_raw_parts(items.as_ptr().cast::(), core::mem::size_of_val(items)) - }; +unsafe fn append_fixed_slice_as_bytes(items: &[T], buf: &mut Vec) { + // Padding bytes in a valid `T` may be uninitialized, and a `&[u8]` + // covering uninitialized bytes is UB (a reference must point to + // valid values, and uninitialized `u8` is invalid). + let byte_slice = + core::slice::from_raw_parts(items.as_ptr().cast::(), core::mem::size_of_val(items)); buf.extend_from_slice(byte_slice); } @@ -101,7 +107,8 @@ impl SszEncode for u8 { fn ssz_append_fixed_slice(items: &[Self], buf: &mut Vec) { #[cfg(target_endian = "little")] { - append_fixed_slice_as_bytes(items, buf); + // SAFETY: primitive integers have no padding. + unsafe { append_fixed_slice_as_bytes(items, buf) }; } #[cfg(not(target_endian = "little"))] { @@ -134,7 +141,8 @@ impl SszEncode for u16 { fn ssz_append_fixed_slice(items: &[Self], buf: &mut Vec) { #[cfg(target_endian = "little")] { - append_fixed_slice_as_bytes(items, buf); + // SAFETY: primitive integers have no padding. + unsafe { append_fixed_slice_as_bytes(items, buf) }; } #[cfg(not(target_endian = "little"))] { @@ -167,7 +175,8 @@ impl SszEncode for u32 { fn ssz_append_fixed_slice(items: &[Self], buf: &mut Vec) { #[cfg(target_endian = "little")] { - append_fixed_slice_as_bytes(items, buf); + // SAFETY: primitive integers have no padding. + unsafe { append_fixed_slice_as_bytes(items, buf) }; } #[cfg(not(target_endian = "little"))] { @@ -200,7 +209,8 @@ impl SszEncode for u64 { fn ssz_append_fixed_slice(items: &[Self], buf: &mut Vec) { #[cfg(target_endian = "little")] { - append_fixed_slice_as_bytes(items, buf); + // SAFETY: primitive integers have no padding. + unsafe { append_fixed_slice_as_bytes(items, buf) }; } #[cfg(not(target_endian = "little"))] { @@ -233,7 +243,8 @@ impl SszEncode for u128 { fn ssz_append_fixed_slice(items: &[Self], buf: &mut Vec) { #[cfg(target_endian = "little")] { - append_fixed_slice_as_bytes(items, buf); + // SAFETY: primitive integers have no padding. + unsafe { append_fixed_slice_as_bytes(items, buf) }; } #[cfg(not(target_endian = "little"))] { @@ -266,7 +277,8 @@ impl SszEncode for [u8; N] { } #[cfg(feature = "alloc")] fn ssz_append_fixed_slice(items: &[Self], buf: &mut Vec) { - append_fixed_slice_as_bytes(items, buf); + // SAFETY: [u8; N] has no padding. + unsafe { append_fixed_slice_as_bytes(items, buf) }; } }