Some unsound usage of unsafe api

[yaokunzhang]

Hello, and thank you for your contribution to the open-source community!

While scanning crates on crates.io using my static analysis tool, I discovered several potential unsafe API usage patterns in this crate, specifically in lib.rs within the ProcessMemory trait.

Here are some unsafe usage patterns that may lead to undefined behavior:

1. copy_struct and copy_pointer

These methods may lead to double-free issues if used with non-POD (plain old data) types. For example:

use remoteprocess::{LocalProcess, ProcessMemory};

#[derive(Debug)]
struct Victim {
    message: String,
}

fn main() {
    let victim = Victim {
        message: "Hello from heap".to_string(),
    };
    let _copy: Victim = LocalProcess.copy_struct(&victim as *const _ as usize).unwrap();
    // Both `victim` and `_copy` will attempt to drop the same heap memory
}

The same issue also applies to copy_pointer, as it simply wraps around copy_struct.

2. copy_vec may lead to double-drop

If the original data (pointed to by the given address) is not explicitly forgotten or handled carefully, calling copy_vec may result in double drop or use-after-free:

use remoteprocess::{LocalProcess, ProcessMemory};

#[derive(Debug)]
struct Victim(String);

fn main() {
    let vec = vec![Victim("boom".to_string())];
    let _copy: Vec<Victim> = LocalProcess.copy_vec(vec.as_ptr() as usize, 1).unwrap();
    // `vec[0]` and `_copy[0]` share the same heap allocation
}

3. Alignment violations

Many methods in this trait assume alignment safety but don’t enforce it explicitly. For example, in copy_vec:

fn copy_vec<T>(&self, addr: usize, length: usize) -> Result<Vec<T>, Error> {
    let mut vec = self.copy(addr, length * std::mem::size_of::<T>())?;
    let capacity = vec.capacity() / std::mem::size_of::<T>();
    let ptr = vec.as_mut_ptr() as *mut T;
    std::mem::forget(vec);
    unsafe { Ok(Vec::from_raw_parts(ptr, capacity, capacity)) }
}

This code reinterprets a Vec<u8> as a Vec<T>, which may violate alignment constraints for T, resulting in undefined behavior on some platforms.

Suggestion

From the Rust type system's perspective, these methods are unsound unless the caller can guarantee the following:

• The memory pointed to is valid and properly aligned for type T.
• The type T is trivially copyable (i.e., POD or Copy) and does not own any resources.
• There are no aliasing or ownership violations during or after the copy.

If these constraints cannot be enforced at the type level, I would suggest marking these methods as unsafe, and documenting the exact invariants that must be upheld by the caller.

I’m not aware of the full intended use cases of this crate, so it's possible that these patterns are safe in practice under certain domain-specific assumptions. However, clarifying those assumptions in the documentation or making the contracts explicit using unsafe would help avoid accidental misuse.

Thanks again for your work!

[benfred]

Thanks for the feedback -

This crate is meant for getting information from other processes - and the assumption was always that we were copying POD data types from the other process. I've added a requirement for objects being copied to have the Copy trait to make this explicit in https://github.com/benfred/remoteprocess/pull/109 .

Likewise the LocalProcess struct is just for the unittests, and is not the focus of this crate, and isn't used by dependencies afaik. This crate is mainly used by py-spy and rbspy for profiling python and ruby processes -

[lewismosciski]

Hi @benfred ,

Thanks for the quick response and adding T: Copy.

While that's a great step, it unfortunately doesn't fully prevent Undefined Behavior. A type can be Copy but still have invalid bit patterns.

The standard way to fix this is to either mark these methods as unsafe (making the safety contract clear to the caller) .

I understand that changing the public API to be unsafe fn is likely not practical given the downstream users.My main goal was just to elaborate on the concept slightly. Please feel free to ignore this if you wish.