scottlamb / moonfire-nvr

Moonfire NVR, a security camera network video recorder
Other
1.2k stars 138 forks source link

`Segment::build_index` is unsound #185

Closed scottlamb closed 1 year ago

scottlamb commented 2 years ago

https://github.com/scottlamb/moonfire-nvr/blob/0406e09ca470d85d71145a9b6d5b88ea95f4b785/server/src/mp4.rs#L447-L448

We're violating the safety assumption of Vec::set_len by calling it before the elements are initialized. This is Undefined Behavior. As far as I know, it's not causing any observable problem today, but that's dependent on the compiler version.

The obvious fix is to use Vec<MaybeUninit<u8>> instead and transmute it later. I'd like to use MaybeUninit::write_slice to extend it, but that's unstable. I might just punt until that changes, but filing this issue so I don't forget entirely.

We could instead use std::ptr::copy_nonoverlapping on a *mut u8, but it'd be nice to keep using bounds checking.