CVE-2023-41051 (Rust, published 2023-09-01T19:15Z)
Description: value: In a typical Virtual Machine Monitor (VMM) there are several components, such as boot loader, virtual device drivers, virtio backend drivers and vhost drivers, that need to access the VM physical memory. The vm-memory rust crate provides a set of traits to decouple VM memory consumers from VM memory providers. An issue was discovered in the default implementations of the `VolatileMemory::{get_atomic_ref, aligned_as_ref, aligned_as_mut, get_ref, get_array_ref}` trait functions, which allows out-of-bounds memory access if the `VolatileMemory::get_slice` function returns a `VolatileSlice` whose length is less than the function’s `count` argument. No implementations of `get_slice` provided in `vm_memory` are affected. Users of custom `VolatileMemory` implementations may be impacted if the custom implementation does not adhere to `get_slice`'s documentation. The issue started in version 0.1.0 but was fixed in version 0.12.2 by inserting a check that verifies that the `VolatileSlice` returned by `get_slice` is of the correct length. Users are advised to upgrade. There are no known workarounds for this issue.

Severity: nan (CVSS v3 base score: 4.7)
Weakness: CWE-125 \u2014 Out-of-bounds Read
The product reads data past the end, or before the beginning, of the intended buffer.
Repository: https://github.com/rust-vmm/vm-memory
Files changed:
CHANGELOG.md
Cargo.toml
src/volatile_memory.rs
Commit message: fix: Validate return value of get_slice in VolatileMemory

An issue was discovered in the default implementations of the
VolatileMemory::{get_atomic_ref, aligned_as_ref, aligned_as_mut,
get_ref, get_array_ref} trait functions, which allows out-of-bounds
memory access if the VolatileMemory::get_slice function returns a
VolatileSlice whose length is less than the function’s count argument.
No implementations of get_slice provided in vm_memory are affected.
Users of custom VolatileMemory implementations may be impacted if the
custom implementation does not adhere to get_slice's documentation.

This commit fixes this issue by inserting a check that verifies that the
VolatileSlice returned by get_slice is of the correct length.

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>


Vulnerable code:
## [Unreleased]
### Added

### Changed
## [Unreleased]

### Deprecated

- [[#244]](https://github.com/rust-vmm/vm-memory/pull/241) Deprecate volatile
  memory's `as_ptr()` interfaces. The new interfaces to be used instead are:
  `ptr_guard()` and `ptr_guard_mut()`.

version = "0.12.1"
        // SAFETY: This is safe because the pointer is range-checked by get_slice, and
        // the lifetime is the same as self.
        // SAFETY: This is safe because the pointer is range-checked by get_slice, and
        // the lifetime is the same as self.
        Ok(&*(slice.addr as *const T))
        Ok(&mut *(slice.addr as *mut T))
        // SAFETY: This is safe because the pointer is range-checked by get_slice, and
        // the lifetime is the same as self.

Fixed code:
## [v0.12.2]
- [[#251]](https://github.com/rust-vmm/vm-memory/pull/251): Inserted checks
  that verify that the value returned by `VolatileMemory::get_slice` is of
  the correct length.
- [[#244]](https://github.com/rust-vmm/vm-memory/pull/241) Deprecate volatile
  memory's `as_ptr()` interfaces. The new interfaces to be used instead are:
  `ptr_guard()` and `ptr_guard_mut()`.
version = "0.12.2"
    ///
    /// Note that the property `get_slice(offset, count).len() == count` MUST NOT be
    /// relied on for the correctness of unsafe code. This is a safe function inside of a
    /// safe trait, and implementors are under no obligation to follow its documentation.

        assert_eq!(
            slice.len(),
            size_of::<T>(),
            "VolatileMemory::get_slice(offset, count) returned slice of length != count."
        );

        // SAFETY: This is safe because the invariants of the constructors of VolatileSlice ensure that
        // slice.addr is valid memory of size slice.len(). The assert above ensures that
        // the length of the slice is exactly enough to hold one `T`. Lastly, the lifetime of the
        // returned VolatileRef match that of the VolatileSlice returned by get_slice and thus the
        // lifetime one `self`.

        assert_eq!(
            slice.len(),
            nbytes as usize,
            "VolatileMemory::get_slice(offset, count) returned slice of length != count."
        );

        // SAFETY: This is safe because the invariants of the constructors of VolatileSlice ensure that
        // slice.addr is valid memory of size slice.len(). The assert above ensures that
        // the length of the slice is exactly enough to hold `n` instances of `T`. Lastly, the lifetime of the
        // returned VolatileArrayRef match that of the VolatileSlice returned by get_slice and thus the
        // lifetime one `self`.

        assert_eq!(
            slice.len(),
            size_of::<T>(),
            "VolatileMemory::get_slice(offset, count) returned slice of length != count."
        );

        // SAFETY: This is safe because the invariants of the constructors of VolatileSlice ensure that
        // slice.addr is valid memory of size slice.len(). The assert above ensures that
        // the length of the slice is exactly enough to hold one `T`.
        // Dereferencing the pointer is safe because we check the alignment above, and the invariants
        // of this function ensure that no aliasing pointers exist. Lastly, the lifetime of the
        // returned VolatileArrayRef match that of the VolatileSlice returned by get_slice and thus the
        // lifetime one `self`.
        unsafe { Ok(&*(slice.addr as *const T)) }
        assert_eq!(
            slice.len(),
            size_of::<T>(),
            "VolatileMemory::get_slice(offset, count) returned slice of length != count."
        );

        // SAFETY: This is safe because the invariants of the constructors of VolatileSlice ensure that
        // slice.addr is valid memory of size slice.len(). The assert above ensures that
        // the length of the slice is exactly enough to hold one `T`.
        // Dereferencing the pointer is safe because we check the alignment above, and the invariants
        // of this function ensure that no aliasing pointers exist. Lastly, the lifetime of the
        // returned VolatileArrayRef match that of the VolatileSlice returned by get_slice and thus the
        // lifetime one `self`.

        unsafe { Ok(&mut *(slice.addr as *mut T)) }
        assert_eq!(
            slice.len(),
            size_of::<T>(),
            "VolatileMemory::get_slice(offset, count) returned slice of length != count."
        );

        // SAFETY: This is safe because the invariants of the constructors of VolatileSlice ensure that
        // slice.addr is valid memory of size slice.len(). The assert above ensures that
        // the length of the slice is exactly enough to hold one `T`.
        // Dereferencing the pointer is safe because we check the alignment above. Lastly, the lifetime of the
        // returned VolatileArrayRef match that of the VolatileSlice returned by get_slice and thus the
        // lifetime one `self`.