search

Rust-SDL2 / rust-sdl2

SDL2 bindings for Rust
MIT License
2.77k stars 474 forks source link

mixer::Music is unsound #616

Open crumblingstatue opened 7 years ago

crumblingstatue commented 7 years ago

It has its internal fields exposed publicly, allowing the user to break its invariants using safe code.

Example:

let mut music = Music::from_file("foo.mid")?;
music.raw = 0xDEADBEEF as *mut _;
let _ = music.play(-1);
Cobrand commented 7 years ago

I think the original author made raw public so that unsafe methods from the ffi could be used, but it's true that allowing to change the raw pointer is clearly undefined behavior ... Even if we assume that the pointer is changed but not played afterwards, drop will still be called on a value that isn't valid, while the other value will effectively be leaked memory.

The solution would be to remove the pub attributes and add a as_raw(&self) -> *mut c_void method if someone really needs the underlying pointer.

I don't know why owned is public either, but it shouldn't be.