Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust soundness fixes #7518

Merged
merged 15 commits into from Sep 29, 2022
3 changes: 2 additions & 1 deletion rust/flatbuffers/src/follow.rs
Expand Up @@ -31,7 +31,8 @@ pub trait Follow<'buf> {
type Inner;
/// # Safety
///
/// `buf[loc..]` must contain a valid value of `Self`
/// `buf[loc..]` must contain a valid value of `Self` and anything it
/// transitively refers to by offset must also be valid
unsafe fn follow(buf: &'buf [u8], loc: usize) -> Self::Inner;
}

Expand Down
4 changes: 2 additions & 2 deletions rust/flatbuffers/src/vector.rs
Expand Up @@ -64,8 +64,8 @@ impl<'a, T: 'a> Vector<'a, T> {
///
/// `buf` contains a valid vector at `loc` consisting of
///
/// UOffsetT element count
/// Consecutive list of `T` elements
/// - UOffsetT element count
/// - Consecutive list of `T` elements
#[inline(always)]
pub unsafe fn new(buf: &'a [u8], loc: usize) -> Self {
Vector(buf, loc, PhantomData)
Expand Down
30 changes: 26 additions & 4 deletions src/idl_gen_rust.cpp
Expand Up @@ -737,7 +737,6 @@ class RustGenerator : public BaseGenerator {
code_ += "pub use self::bitflags_{{ENUM_NAMESPACE}}::{{ENUM_TY}};";
code_ += "";

code_.SetValue("FROM_BASE", "Self::from_bits_truncate(b)");
code_.SetValue("INTO_BASE", "self.bits()");
} else {
// Normal, c-modelled enums.
Expand Down Expand Up @@ -810,7 +809,6 @@ class RustGenerator : public BaseGenerator {
code_ += " }";
code_ += "}";

code_.SetValue("FROM_BASE", "Self(b)");
code_.SetValue("INTO_BASE", "self.0");
}

Expand Down Expand Up @@ -841,7 +839,19 @@ class RustGenerator : public BaseGenerator {
code_ += " #[inline]";
code_ += " unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {";
code_ += " let b = flatbuffers::read_scalar_at::<{{BASE_TYPE}}>(buf, loc);";
code_ += " {{FROM_BASE}}";
if (IsBitFlagsEnum(enum_def)) {
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
code_ += " // Safety:";
CasperN marked this conversation as resolved.
Show resolved Hide resolved
code_ += " // This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.";
code_ += " // from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0";
code_ += " // https://github.com/bitflags/bitflags/issues/262";
code_ += " Self::from_bits_unchecked(b)";
} else {
code_ += " Self(b)";
}
code_ += " }";
code_ += "}";
code_ += "";
Expand All @@ -863,7 +873,19 @@ class RustGenerator : public BaseGenerator {
code_ += " #[allow(clippy::wrong_self_convention)]";
code_ += " fn from_little_endian(v: {{BASE_TYPE}}) -> Self {";
code_ += " let b = {{BASE_TYPE}}::from_le(v);";
code_ += " {{FROM_BASE}}";
if (IsBitFlagsEnum(enum_def)) {
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
code_ += " // Safety:";
code_ += " // This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.";
code_ += " // from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0";
code_ += " // https://github.com/bitflags/bitflags/issues/262";
code_ += " unsafe { Self::from_bits_unchecked(b) }";
} else {
code_ += " Self(b)";
}
code_ += " }";
code_ += "}";
code_ += "";
Expand Down
12 changes: 10 additions & 2 deletions tests/monster_test/my_game/example/color_generated.rs
Expand Up @@ -31,7 +31,11 @@ impl<'a> flatbuffers::Follow<'a> for Color {
#[inline]
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let b = flatbuffers::read_scalar_at::<u8>(buf, loc);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
Self::from_bits_unchecked(b)
}
}

Expand All @@ -53,7 +57,11 @@ impl flatbuffers::EndianScalar for Color {
#[allow(clippy::wrong_self_convention)]
fn from_little_endian(v: u8) -> Self {
let b = u8::from_le(v);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
unsafe { Self::from_bits_unchecked(b) }
}
}

Expand Down
12 changes: 10 additions & 2 deletions tests/monster_test/my_game/example/long_enum_generated.rs
Expand Up @@ -27,7 +27,11 @@ impl<'a> flatbuffers::Follow<'a> for LongEnum {
#[inline]
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let b = flatbuffers::read_scalar_at::<u64>(buf, loc);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
Self::from_bits_unchecked(b)
}
}

Expand All @@ -49,7 +53,11 @@ impl flatbuffers::EndianScalar for LongEnum {
#[allow(clippy::wrong_self_convention)]
fn from_little_endian(v: u64) -> Self {
let b = u64::from_le(v);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
unsafe { Self::from_bits_unchecked(b) }
}
}

Expand Down
12 changes: 10 additions & 2 deletions tests/monster_test_serialize/my_game/example/color_generated.rs
Expand Up @@ -42,7 +42,11 @@ impl<'a> flatbuffers::Follow<'a> for Color {
#[inline]
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let b = flatbuffers::read_scalar_at::<u8>(buf, loc);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
Self::from_bits_unchecked(b)
}
}

Expand All @@ -64,7 +68,11 @@ impl flatbuffers::EndianScalar for Color {
#[allow(clippy::wrong_self_convention)]
fn from_little_endian(v: u8) -> Self {
let b = u8::from_le(v);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
unsafe { Self::from_bits_unchecked(b) }
}
}

Expand Down
Expand Up @@ -38,7 +38,11 @@ impl<'a> flatbuffers::Follow<'a> for LongEnum {
#[inline]
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let b = flatbuffers::read_scalar_at::<u64>(buf, loc);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
Self::from_bits_unchecked(b)
}
}

Expand All @@ -60,7 +64,11 @@ impl flatbuffers::EndianScalar for LongEnum {
#[allow(clippy::wrong_self_convention)]
fn from_little_endian(v: u64) -> Self {
let b = u64::from_le(v);
Self::from_bits_truncate(b)
// Safety:
// This is safe because we know bitflags is implemented with a repr transparent uint of the correct size.
// from_bits_unchecked will be replaced by an equivalent but safe from_bits_retain in bitflags 2.0
// https://github.com/bitflags/bitflags/issues/262
unsafe { Self::from_bits_unchecked(b) }
}
}

Expand Down