From cfb50dc41e935d7a05a80423a7e3b2bbc28d6088 Mon Sep 17 00:00:00 2001 From: mooncitydev Date: Thu, 18 Jun 2026 16:48:02 +0900 Subject: [PATCH] fix(wincode): stop serializing uninitialized podstring/podvec capacity PodString and PodVec keep their tail in a [MaybeUninit<_>; N] capacity array, so only the length prefix and the first `len` bytes are ever initialized. the SchemaWrite impls cast the whole struct to &[u8] over size_of::(), which reads the uninitialized capacity past `len`. that is UB (Miri flags it as a read of uninitialized memory), it leaks stale stack/heap bytes into the serialized output, and it makes serialization non-deterministic: two equal values can encode to different bytes depending on whatever was left in the capacity, which breaks the byte stability the crate otherwise upholds. write the initialized prefix + active bytes and zero-fill the rest of the capacity, so the wire image is still a fixed size_of::() bytes but fully initialized and deterministic. the read side is unchanged since it only ever looks at the first `len` bytes. --- zeropod/src/pod/wincode.rs | 64 ++++++++++++++++++++++------ zeropod/tests/wincode_serialize.rs | 68 ++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 zeropod/tests/wincode_serialize.rs diff --git a/zeropod/src/pod/wincode.rs b/zeropod/src/pod/wincode.rs index 017aee8..b49d3da 100644 --- a/zeropod/src/pod/wincode.rs +++ b/zeropod/src/pod/wincode.rs @@ -2,6 +2,16 @@ //! derive (generic params, `MaybeUninit` fields). Each impl serializes the //! full fixed-size byte representation via raw pointer cast — matching the //! zero-copy layout used on-chain. +//! +//! `PodString` and `PodVec` keep their dynamic data in a `[MaybeUninit<_>; N]` +//! capacity array: only the length prefix and the first `len` bytes are +//! initialized, the rest of the capacity is uninitialized. Serializing them by +//! casting the whole struct to `&[u8]` would read that uninitialized capacity, +//! which is undefined behavior and also leaks stale stack/heap bytes into the +//! output (making it non-deterministic). Instead we write the initialized +//! prefix + active bytes and zero-fill the remaining capacity, so the on-wire +//! image stays a fixed `size_of::()` bytes but is fully initialized and +//! deterministic. use { super::{option::PodOption, string::PodString, vec::PodVec}, @@ -9,6 +19,22 @@ use { wincode::config::ConfigCore, }; +/// Write `n` zero bytes without allocating, used to pad a fixed-size pod image +/// out to its declared `size_of` once the initialized bytes have been written. +#[inline] +fn write_zeroed_padding( + mut writer: impl wincode::io::Writer, + mut n: usize, +) -> wincode::error::WriteResult<()> { + const ZEROS: [u8; 64] = [0u8; 64]; + while n > 0 { + let chunk = n.min(ZEROS.len()); + writer.write(&ZEROS[..chunk])?; + n -= chunk; + } + Ok(()) +} + // --------------------------------------------------------------------------- // PodString // --------------------------------------------------------------------------- @@ -26,13 +52,18 @@ unsafe impl wincode::SchemaWrit mut __writer: impl wincode::io::Writer, src: &Self, ) -> wincode::error::WriteResult<()> { - let __bytes = unsafe { - core::slice::from_raw_parts( - src as *const Self as *const u8, - core::mem::size_of::(), - ) - }; - __writer.write(__bytes)?; + // Initialized region is the PFX-byte length prefix followed by the + // first `len` data bytes; both live contiguously at the front of the + // (align-1, padding-free) struct. + let __init_len = PFX + src.len(); + // SAFETY: `src` is `#[repr(C)]` align 1, so the length prefix occupies + // bytes `[0, PFX)` and the active data `[PFX, PFX + len)`, all + // initialized and contiguous. `len()` is clamped to `N`, so + // `__init_len <= size_of::()`. + let __init = + unsafe { core::slice::from_raw_parts(src as *const Self as *const u8, __init_len) }; + __writer.write(__init)?; + write_zeroed_padding(__writer.by_ref(), core::mem::size_of::() - __init_len)?; Ok(()) } } @@ -72,13 +103,18 @@ unsafe impl wincode: mut __writer: impl wincode::io::Writer, src: &Self, ) -> wincode::error::WriteResult<()> { - let __bytes = unsafe { - core::slice::from_raw_parts( - src as *const Self as *const u8, - core::mem::size_of::(), - ) - }; - __writer.write(__bytes)?; + // Initialized region is the PFX-byte length prefix followed by the + // first `len` elements. `T: ZcElem` is align 1, so elements are packed + // with no padding and sit contiguously after the prefix. + let __init_len = PFX + src.len() * core::mem::size_of::(); + // SAFETY: `src` is `#[repr(C)]` align 1; the prefix occupies `[0, PFX)` + // and the active elements `[PFX, PFX + len * size_of::())`, all + // initialized and contiguous. `len()` is clamped to `N`, so + // `__init_len <= size_of::()`. + let __init = + unsafe { core::slice::from_raw_parts(src as *const Self as *const u8, __init_len) }; + __writer.write(__init)?; + write_zeroed_padding(__writer.by_ref(), core::mem::size_of::() - __init_len)?; Ok(()) } } diff --git a/zeropod/tests/wincode_serialize.rs b/zeropod/tests/wincode_serialize.rs new file mode 100644 index 0000000..4039e4d --- /dev/null +++ b/zeropod/tests/wincode_serialize.rs @@ -0,0 +1,68 @@ +//! Regression tests for wincode `SchemaWrite` on `PodString` / `PodVec`. +//! +//! These containers keep their data in a `[MaybeUninit<_>; N]` capacity array, +//! so only the length prefix and the first `len` bytes are initialized. The +//! serializer used to cast the whole struct to `&[u8]`, which read the +//! uninitialized capacity (UB under Miri) and leaked stale bytes into the +//! output, making serialization non-deterministic. Serialization must now emit +//! a fixed-size image that is fully initialized and depends only on the logical +//! value. +#![cfg(feature = "wincode")] + +use zeropod::pod::{PodString, PodU16, PodVec}; + +#[test] +fn serialize_partial_podstring_zero_pads_capacity_and_roundtrips() { + let mut s = PodString::<8>::default(); + assert!(s.set("hi")); + + let bytes = wincode::serialize(&s).unwrap(); + + // PFX(1) + N(8) = 9 bytes: [len][active][zeroed capacity]. + assert_eq!(bytes.len(), core::mem::size_of::>()); + assert_eq!(bytes, [2, b'h', b'i', 0, 0, 0, 0, 0, 0]); + + let back = wincode::deserialize::>(&bytes).unwrap(); + assert_eq!(back.as_str(), "hi"); +} + +#[test] +fn serialize_partial_podvec_zero_pads_capacity_and_roundtrips() { + let mut v = PodVec::::default(); + assert!(v.push(PodU16::from(5))); + assert!(v.push(PodU16::from(7))); + + let bytes = wincode::serialize(&v).unwrap(); + + // PFX(2) + N(4)*size_of(PodU16)(2) = 10 bytes. + assert_eq!(bytes.len(), core::mem::size_of::>()); + assert_eq!(bytes, [2, 0, 5, 0, 7, 0, 0, 0, 0, 0]); + + let back = wincode::deserialize::>(&bytes).unwrap(); + assert_eq!(back.len(), 2); + assert_eq!(back.as_slice(), &[PodU16::from(5), PodU16::from(7)]); +} + +#[test] +fn serialize_is_deterministic_regardless_of_stale_capacity() { + // `truncated` once held a longer value, so its capacity tail still contains + // the old "defgh" bytes. `fresh` only ever held "abc". Both represent the + // same logical string and must serialize to identical bytes. + let mut truncated = PodString::<16>::default(); + assert!(truncated.set("abcdefgh")); + truncated.truncate(3); + assert_eq!(truncated.as_str(), "abc"); + + let mut fresh = PodString::<16>::default(); + assert!(fresh.set("abc")); + + let a = wincode::serialize(&truncated).unwrap(); + let b = wincode::serialize(&fresh).unwrap(); + assert_eq!( + a, b, + "serialization must not depend on stale capacity bytes" + ); + + // And the stale "defgh" must not leak into the output. + assert!(!a[4..].iter().any(|&byte| byte != 0)); +}