search

rust-vmm / vm-memory

Virtual machine's guest memory crate
Apache License 2.0
299 stars 97 forks source link

Unsoundness: VolatileMemory should be `unsafe`, private, or sealed because it relies on get_slice in unsafe code #250

Closed Manishearth closed 12 months ago

Manishearth commented 1 year ago

https://github.com/rust-vmm/vm-memory/blob/06ebc2a36e1df29923887cb32a829ad6ea243634/src/volatile_memory.rs#L110-L112

The behavior of this trait method is relied upon in multiple places in unsafe code, e.g:

https://github.com/rust-vmm/vm-memory/blob/06ebc2a36e1df29923887cb32a829ad6ea243634/src/volatile_memory.rs#L122-L123

A user of this library implementing VolatileMemory could cause unsoundness by writing an incorrect implementation of VolatileMemory::get_slice(). This means that it should be an unsafe trait (or private/sealed) with the safety requirements for implementors documented on the trait.

roypat commented 1 year ago

Thank you for bringing your security concerns to our attention! We will investigate these immediately and follow up with you within 5 business days to provide a status.

If you have any further questions or concerns, please do not hesitate to contact us according to the rust-vmm security disclosure policy.

Thanks again!

Manishearth commented 1 year ago

Thanks! I'm not in a rush.

(I did consider using the security policy but didn't in the end since this is basically "someone can cause UB if they use your library weirdly" which is bad from a rust unsafe model perspective but basically ok from a security perspective.)

roypat commented 12 months ago

Hello @Manishearth, we have fixed the issue in #251 and published vm-memory 0.12.2 to crates.io. We will also go through the process of filing a RustSec advisory for affected versions. Thank you again for reporting this issue!