Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsoundness in safe 'copy' function may cause UB due to lack of bounds checks #72

Open
lwz23 opened this issue Nov 27, 2024 · 1 comment

Comments

@lwz23
Copy link

lwz23 commented Nov 27, 2024

Description:
The copy function uses unsafe operations to copy memory from a source address to a destination address. However, the implementation does not perform any bounds checking to ensure the safety of the operation. This can lead to Undefined Behavior (UB) under various scenarios, such as invalid pointers, overlapping memory regions, or out-of-bounds access.

pub fn copy(&self, src: u32, dst: u32, len: u32) {

pub fn copy(&self, src: u32, dst: u32, len: u32) {
    unsafe {
        let src = self.get_ptr::<u8>(src);
        let dst = self.get_ptr_mut::<u8>(dst);
        std::ptr::copy(src, dst, len as usize);
    }
}

Problem Description:

  1. Unchecked pointers:
    The function relies on self.get_ptr and self.get_ptr_mut to provide raw pointers for the source and destination.
    If these pointers are invalid or out of bounds, dereferencing them in std::ptr::copy will lead to UB.
  2. No overlap check:
    The std::ptr::copy function requires that the source and destination memory regions do not overlap. If they overlap, the behavior is undefined.
  3. Unsafe cast of len:
    The len parameter is cast from u32 to usize without validation. On 64-bit systems, this could lead to truncation or incorrect behavior if len exceeds the maximum value of usize.
  4. General safety concerns:
    There is no guarantee that the provided source or destination addresses are valid for reading or writing for the specified len bytes.
    The function assumes that all memory is properly aligned and accessible, which may not be true.

Expected Behavior:

  1. The function should ensure that the pointers provided by self.get_ptr and self.get_ptr_mut are valid and within the allowed memory bounds.
  2. The function should validate that the source and destination memory regions do not overlap.
  3. Proper checks should be implemented to ensure that the len parameter does not exceed memory limits or lead to truncation on 64-bit systems.

Additional Notes:
Unsafe memory operations require rigorous validation to ensure correctness and avoid UB.
If performance is critical, consider providing a safe wrapper around this function to enforce the necessary checks while keeping the unsafe implementation for advanced use cases.

@lwz23
Copy link
Author

lwz23 commented Dec 1, 2024

ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant