Rust: read_scalar(_at) is unsound #5825
Description
The read_scalar and read_scalar_at functions are unsound because the allow to do things such as:
flatbuffers::read_scalar::<bool>(&[3]));or
fn main() {
#[derive(Copy, Clone, PartialEq, Debug)]
struct Nz(std::num::NonZeroI32);
impl flatbuffers::EndianScalar for Nz {
fn to_little_endian(self) -> Self { self }
fn from_little_endian(self) -> Self { self }
}
println!("{:?}", flatbuffers::read_scalar::<Nz>(&[0, 0, 0, 0]));
}or even worse:
fn main() {
#[derive(Copy, Clone, PartialEq, Debug)]
struct S(&'static str);
impl flatbuffers::EndianScalar for S {
fn to_little_endian(self) -> Self { self }
fn from_little_endian(self) -> Self { self }
}
println!("{:?}", flatbuffers::read_scalar::<S>(&[1; std::mem::size_of::<S>()]));
}(this last one causes a segmentation fault)
read_scalar is behaving similar to transmute, leading to undefined behavior for types where not all bit patterns are valid (such as bool or NonZero*) or allowing to create dangling pointers.
I suggest three breaking solutions:
- Make
read_scalarandread_scalar_atfunctions unsafe. - Make the
EndianScalartrait unsafe. - Make the
EndianScalartrait something like:
pub trait EndianScalar {
fn emplace_little_endian(self, buf: &mut [u8]);
fn read_little_endian(buf: &[u8]) -> Self;
}which, for example, would be implemented for u32 like this:
impl EndianScalar for u32 {
fn emplace_little_endian(self, buf: &mut [u8]) {
buf[..4].copy_from_slice(&self.to_le_bytes());
}
fn read_little_endian(buf: &[u8]) -> u32 {
u32::from_le_bytes([buf[0], buf[1], buf[2], buf[3]])
}
}This last solution, even though it is the most disrupting one, has the advantage of not requiring any unsafe at all.