CVE-2023-50711 (Rust, published 2024-01-02T20:15Z)
Description: value: vmm-sys-util is a collection of modules that provides helpers and utilities used by multiple rust-vmm components. Starting in version 0.5.0 and prior to version 0.12.0, an issue in the `FamStructWrapper::deserialize` implementation provided by the crate for `vmm_sys_util::fam::FamStructWrapper` can lead to out of bounds memory accesses. The deserialization does not check that the length stored in the header matches the flexible array length. Mismatch in the lengths might allow out of bounds memory access through Rust-safe methods. The issue was corrected in version 0.12.0 by inserting a check that verifies the lengths of compared flexible arrays are equal for any deserialized header and aborting deserialization otherwise. Moreover, the API was changed so that header length can only be modified through Rust-unsafe code. This ensures that users cannot trigger out-of-bounds memory access from Rust-safe code.
Severity: nan (CVSS v3 base score: 9.8)
Weakness: CWE-787 \u2014 Out-of-bounds Write
The product writes data past the end, or before the beginning, of the intended buffer.
Repository: https://github.com/rust-vmm/vmm-sys-util
Files changed:
Cargo.toml
src/fam.rs
Commit message: fix: deserialization issue of FamStructWrapper with serde

An issue was discovered in the Serde::deserialize implementation of
the FamStructWrapper that can lead to out-of-bounds memory access via
safe Rust code.

When dserializing a FamStructWrapper we reconstruct the header of the
type from the saved state and then reconstruct the flexible array part
in a separate step. The header includes information about the length of
the flexible array part. However, during deserialization, we do not
check that the length included in the header matches with the length of
the deserialized flexible array. The safety of FamStructWrapper methods
accessing the underlying memory of the flexible array depends on the
header length reflects the memory size of the flexible array.

If the saved state was malformed in way that this condition is not true,
and even worse, the header length implies a flexible array buffer bigger
than what we allocated memory for, a user can trigger out-bounds access
via Rust-safe code.

This commit introduces a check that the header length matches the length
of the flexible array deserialized from the saved state. If it doesn't,
deserialization returns an Error.

Moreover, we mark the method that can change the header length as unsafe
and make the as_mut_fam_struct method private, so that it is not
possible for a consumer of the library to break this invariant from safe
code.

Signed-off-by: Babis Chalios <bchalios@amazon.es>


Vulnerable code:
version = "0.11.2"
///     fn set_len(&mut self, len: usize) {
    fn set_len(&mut self, len: usize);
        mem_allocator[0].set_len(num_elements);
    /// Get a mut reference to the actual [`FamStruct`](trait.FamStruct.html) instance.
    pub fn as_mut_fam_struct(&mut self) -> &mut T {
        self.as_mut_fam_struct().set_len(len);
                let header = seq
            fn set_len(&mut self, len: usize) {
    #[derive(Default, PartialEq, Eq)]

Fixed code:
version = "0.12.0"
bincode = "1.3.3"
///     unsafe fn set_len(&mut self, len: usize) {
    ///
    /// # Safety
    ///
    /// The caller needs to ensure that `len` here reflects the correct number of entries of the
    /// flexible array part of the struct.
    unsafe fn set_len(&mut self, len: usize);
        // SAFETY: The flexible array part of the struct has `num_elements` capacity. We just
        // initialized this in `mem_allocator`.
        unsafe {
            mem_allocator[0].set_len(num_elements);
        }
    // Get a mut reference to the actual [`FamStruct`](trait.FamStruct.html) instance.
    fn as_mut_fam_struct(&mut self) -> &mut T {
        // SAFETY: We just adjusted the memory for the underlying `mem_allocator` to hold `len`
        // entries.
        unsafe {
            self.as_mut_fam_struct().set_len(len);
        }
                let header: X = seq
                if header.len() != entries.len() {
                    let msg = format!(
                        "Mismatch between length of FAM specified in FamStruct header ({}) \
                         and actual size of FAM ({})",
                        header.len(),
                        entries.len()
                    );
                    return Err(V::Error::custom(msg));
                }

            unsafe fn set_len(&mut self, len: usize) {
    #[derive(Default, Debug, PartialEq, Eq)]

    #[cfg(feature = "with-serde")]
    #[test]
    fn test_bad_deserialize() {
        #[repr(C)]
        #[derive(Default, Debug, PartialEq, Serialize, Deserialize)]
        struct Foo {
            pub len: u32,
            pub padding: u32,
            pub entries: __IncompleteArrayField<u32>,
        }

        generate_fam_struct_impl!(Foo, u32, entries, u32, len, 100);

        let state = FamStructWrapper::<Foo>::new(0).unwrap();
        let mut bytes = bincode::serialize(&state).unwrap();

        // The `len` field of the header is the first to be serialized.
        // Writing at position 0 of the serialized data should change its value.
        bytes[0] = 255;

        assert!(
            matches!(bincode::deserialize::<FamStructWrapper<Foo>>(&bytes).map_err(|boxed| *boxed), Err(bincode::ErrorKind::Custom(s)) if s == *"Mismatch between length of FAM specified in FamStruct header (255) and actual size of FAM (0)")
        );
    }