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
22 changes: 8 additions & 14 deletions rust/flatbuffers/src/array.rs
Expand Up @@ -38,7 +38,7 @@ where
#[allow(clippy::from_over_into)] // TODO(caspern): Go from From to Into.
impl<'a, T: 'a, const N: usize> Array<'a, T, N> {
#[inline(always)]
pub fn new(buf: &'a [u8]) -> Self {
pub unsafe fn new(buf: &'a [u8]) -> Self {
assert!(size_of::<T>() * N == buf.len());

Array {
Expand All @@ -61,12 +61,12 @@ impl<'a, T: Follow<'a> + 'a, const N: usize> Array<'a, T, N> {
pub fn get(&self, idx: usize) -> T::Inner {
assert!(idx < N);
let sz = size_of::<T>();
T::follow(self.0, sz * idx)
unsafe { T::follow(self.0, sz * idx) }
}

#[inline(always)]
pub fn iter(&self) -> VectorIter<'a, T> {
VectorIter::from_slice(self.0, self.len())
unsafe { VectorIter::from_slice(self.0, self.len()) }
}
}

Expand All @@ -83,27 +83,21 @@ impl<'a, T: Follow<'a> + Debug, const N: usize> Into<[T::Inner; N]> for Array<'a
impl<'a, T: Follow<'a> + 'a, const N: usize> Follow<'a> for Array<'a, T, N> {
type Inner = Array<'a, T, N>;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
tustvold marked this conversation as resolved.
Show resolved Hide resolved
Array::new(&buf[loc..loc + N * size_of::<T>()])
}
}

pub fn emplace_scalar_array<T: EndianScalar, const N: usize>(
pub unsafe fn emplace_scalar_array<T: EndianScalar, const N: usize>(
buf: &mut [u8],
loc: usize,
src: &[T; N],
) {
let mut buf_ptr = buf[loc..].as_mut_ptr();
for item in src.iter() {
let item_le = item.to_little_endian();
unsafe {
core::ptr::copy_nonoverlapping(
&item_le as *const T as *const u8,
buf_ptr,
size_of::<T>(),
);
buf_ptr = buf_ptr.add(size_of::<T>());
}
core::ptr::copy_nonoverlapping(&item_le as *const T as *const u8, buf_ptr, size_of::<T>());
buf_ptr = buf_ptr.add(size_of::<T>());
}
}

Expand Down Expand Up @@ -134,7 +128,7 @@ where
}
}

#[cfg(feature="serialize")]
#[cfg(feature = "serialize")]
impl<'a, T: 'a, const N: usize> serde::ser::Serialize for Array<'a, T, N>
where
T: 'a + Follow<'a>,
Expand Down
22 changes: 12 additions & 10 deletions rust/flatbuffers/src/builder.rs
Expand Up @@ -16,13 +16,13 @@

extern crate smallvec;

#[cfg(feature = "no_std")]
use alloc::{vec, vec::Vec};
use core::cmp::max;
use core::iter::{DoubleEndedIterator, ExactSizeIterator};
use core::marker::PhantomData;
use core::ptr::write_bytes;
use core::slice::from_raw_parts;
#[cfg(feature = "no_std")]
use alloc::{vec, vec::Vec};

use crate::endian_scalar::{emplace_scalar, read_scalar_at};
use crate::primitives::*;
Expand Down Expand Up @@ -153,7 +153,7 @@ impl<'fbb> FlatBufferBuilder<'fbb> {
self.make_space(sz);
{
let (dst, rest) = (&mut self.owned_buf[self.head..]).split_at_mut(sz);
x.push(dst, rest);
unsafe { x.push(dst, rest) };
tustvold marked this conversation as resolved.
Show resolved Hide resolved
}
WIPOffset::new(self.used_space() as UOffsetT)
}
Expand Down Expand Up @@ -443,7 +443,7 @@ impl<'fbb> FlatBufferBuilder<'fbb> {
assert_msg_name: &'static str,
) {
let idx = self.used_space() - tab_revloc.value() as usize;
let tab = Table::new(&self.owned_buf[self.head..], idx);
let tab = unsafe { Table::new(&self.owned_buf[self.head..], idx) };
let o = tab.vtable().get(slot_byte_loc) as usize;
assert!(o != 0, "missing required field {}", assert_msg_name);
}
Expand Down Expand Up @@ -560,11 +560,13 @@ impl<'fbb> FlatBufferBuilder<'fbb> {
}
}
let new_vt_bytes = &self.owned_buf[vt_start_pos..vt_end_pos];
let found = self.written_vtable_revpos.binary_search_by(|old_vtable_revpos: &UOffsetT| {
let old_vtable_pos = self.owned_buf.len() - *old_vtable_revpos as usize;
let old_vtable = VTable::init(&self.owned_buf, old_vtable_pos);
new_vt_bytes.cmp(old_vtable.as_bytes())
});
let found = self
.written_vtable_revpos
.binary_search_by(|old_vtable_revpos: &UOffsetT| {
let old_vtable_pos = self.owned_buf.len() - *old_vtable_revpos as usize;
let old_vtable = unsafe { VTable::init(&self.owned_buf, old_vtable_pos) };
new_vt_bytes.cmp(old_vtable.as_bytes())
});
let final_vtable_revpos = match found {
Ok(i) => {
// The new vtable is a duplicate so clear it.
Expand All @@ -586,7 +588,7 @@ impl<'fbb> FlatBufferBuilder<'fbb> {
unsafe {
emplace_scalar::<SOffsetT>(
&mut self.owned_buf[table_pos..table_pos + SIZE_SOFFSET],
final_vtable_revpos as SOffsetT - object_revloc_to_vtable.value() as SOffsetT
final_vtable_revpos as SOffsetT - object_revloc_to_vtable.value() as SOffsetT,
);
}

Expand Down
6 changes: 3 additions & 3 deletions rust/flatbuffers/src/follow.rs
Expand Up @@ -29,7 +29,7 @@ use core::marker::PhantomData;
/// continue traversing the FlatBuffer.
pub trait Follow<'buf> {
type Inner;
fn follow(buf: &'buf [u8], loc: usize) -> Self::Inner;
unsafe fn follow(buf: &'buf [u8], loc: usize) -> Self::Inner;
}

/// FollowStart wraps a Follow impl in a struct type. This can make certain
Expand All @@ -42,14 +42,14 @@ impl<'a, T: Follow<'a> + 'a> FollowStart<T> {
Self { 0: PhantomData }
}
#[inline]
pub fn self_follow(&'a self, buf: &'a [u8], loc: usize) -> T::Inner {
pub unsafe fn self_follow(&'a self, buf: &'a [u8], loc: usize) -> T::Inner {
T::follow(buf, loc)
}
}
impl<'a, T: Follow<'a>> Follow<'a> for FollowStart<T> {
type Inner = T::Inner;
#[inline]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
T::follow(buf, loc)
}
}
40 changes: 19 additions & 21 deletions rust/flatbuffers/src/primitives.rs
Expand Up @@ -135,19 +135,17 @@ impl<T> Push for WIPOffset<T> {
type Output = ForwardsUOffset<T>;

#[inline(always)]
fn push(&self, dst: &mut [u8], rest: &[u8]) {
unsafe fn push(&self, dst: &mut [u8], rest: &[u8]) {
let n = (SIZE_UOFFSET + rest.len() - self.value() as usize) as UOffsetT;
unsafe {
emplace_scalar::<UOffsetT>(dst, n);
}
emplace_scalar::<UOffsetT>(dst, n);
}
}

impl<T> Push for ForwardsUOffset<T> {
type Output = Self;

#[inline(always)]
fn push(&self, dst: &mut [u8], rest: &[u8]) {
unsafe fn push(&self, dst: &mut [u8], rest: &[u8]) {
self.value().push(dst, rest);
}
}
Expand Down Expand Up @@ -179,9 +177,9 @@ impl<T> ForwardsUOffset<T> {
impl<'a, T: Follow<'a>> Follow<'a> for ForwardsUOffset<T> {
type Inner = T::Inner;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let slice = &buf[loc..loc + SIZE_UOFFSET];
let off = unsafe { read_scalar::<u32>(slice) as usize };
let off = read_scalar::<u32>(slice) as usize;
T::follow(buf, loc + off)
}
}
Expand All @@ -200,9 +198,9 @@ impl<T> ForwardsVOffset<T> {
impl<'a, T: Follow<'a>> Follow<'a> for ForwardsVOffset<T> {
type Inner = T::Inner;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let slice = &buf[loc..loc + SIZE_VOFFSET];
let off = unsafe { read_scalar::<VOffsetT>(slice) as usize };
let off = read_scalar::<VOffsetT>(slice) as usize;
T::follow(buf, loc + off)
}
}
Expand All @@ -211,7 +209,7 @@ impl<T> Push for ForwardsVOffset<T> {
type Output = Self;

#[inline]
fn push(&self, dst: &mut [u8], rest: &[u8]) {
unsafe fn push(&self, dst: &mut [u8], rest: &[u8]) {
self.value().push(dst, rest);
}
}
Expand All @@ -230,9 +228,9 @@ impl<T> BackwardsSOffset<T> {
impl<'a, T: Follow<'a>> Follow<'a> for BackwardsSOffset<T> {
type Inner = T::Inner;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let slice = &buf[loc..loc + SIZE_SOFFSET];
let off = unsafe { read_scalar::<SOffsetT>(slice) };
let off = read_scalar::<SOffsetT>(slice);
T::follow(buf, (loc as SOffsetT - off) as usize)
}
}
Expand All @@ -241,7 +239,7 @@ impl<T> Push for BackwardsSOffset<T> {
type Output = Self;

#[inline]
fn push(&self, dst: &mut [u8], rest: &[u8]) {
unsafe fn push(&self, dst: &mut [u8], rest: &[u8]) {
self.value().push(dst, rest);
}
}
Expand All @@ -252,7 +250,7 @@ pub struct SkipSizePrefix<T>(PhantomData<T>);
impl<'a, T: Follow<'a> + 'a> Follow<'a> for SkipSizePrefix<T> {
type Inner = T::Inner;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
T::follow(buf, loc + SIZE_SIZEPREFIX)
}
}
Expand All @@ -263,7 +261,7 @@ pub struct SkipRootOffset<T>(PhantomData<T>);
impl<'a, T: Follow<'a> + 'a> Follow<'a> for SkipRootOffset<T> {
type Inner = T::Inner;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
T::follow(buf, loc + SIZE_UOFFSET)
}
}
Expand All @@ -274,7 +272,7 @@ pub struct FileIdentifier;
impl<'a> Follow<'a> for FileIdentifier {
type Inner = &'a [u8];
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
&buf[loc..loc + FILE_IDENTIFIER_LENGTH]
}
}
Expand All @@ -286,16 +284,16 @@ pub struct SkipFileIdentifier<T>(PhantomData<T>);
impl<'a, T: Follow<'a> + 'a> Follow<'a> for SkipFileIdentifier<T> {
type Inner = T::Inner;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
T::follow(buf, loc + FILE_IDENTIFIER_LENGTH)
}
}

impl<'a> Follow<'a> for bool {
type Inner = bool;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe { read_scalar_at::<u8>(buf, loc) != 0 }
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
read_scalar_at::<u8>(buf, loc) != 0
}
}

Expand All @@ -309,8 +307,8 @@ macro_rules! impl_follow_for_endian_scalar {
impl<'a> Follow<'a> for $ty {
type Inner = $ty;
#[inline(always)]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe { read_scalar_at::<$ty>(buf, loc) }
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
read_scalar_at::<$ty>(buf, loc)
}
}
};
Expand Down
8 changes: 3 additions & 5 deletions rust/flatbuffers/src/push.rs
Expand Up @@ -24,7 +24,7 @@ use crate::endian_scalar::emplace_scalar;
/// types.
pub trait Push: Sized {
type Output;
fn push(&self, dst: &mut [u8], _rest: &[u8]);
unsafe fn push(&self, dst: &mut [u8], _rest: &[u8]);
#[inline]
fn size() -> usize {
size_of::<Self::Output>()
Expand Down Expand Up @@ -60,10 +60,8 @@ macro_rules! impl_push_for_endian_scalar {
type Output = $ty;

#[inline]
fn push(&self, dst: &mut [u8], _rest: &[u8]) {
unsafe {
emplace_scalar::<$ty>(dst, *self);
}
unsafe fn push(&self, dst: &mut [u8], _rest: &[u8]) {
emplace_scalar::<$ty>(dst, *self);
}
}
};
Expand Down
14 changes: 7 additions & 7 deletions rust/flatbuffers/src/table.rs
Expand Up @@ -20,18 +20,18 @@ use crate::vtable::VTable;

#[derive(Clone, Copy, Debug, PartialEq)]
pub struct Table<'a> {
pub buf: &'a [u8],
pub loc: usize,
buf: &'a [u8],
tustvold marked this conversation as resolved.
Show resolved Hide resolved
loc: usize,
}

impl<'a> Table<'a> {
#[inline]
pub fn new(buf: &'a [u8], loc: usize) -> Self {
pub unsafe fn new(buf: &'a [u8], loc: usize) -> Self {
Table { buf, loc }
}
#[inline]
pub fn vtable(&self) -> VTable<'a> {
<BackwardsSOffset<VTable<'a>>>::follow(self.buf, self.loc)
unsafe { <BackwardsSOffset<VTable<'a>>>::follow(self.buf, self.loc) }
}
#[inline]
pub fn get<T: Follow<'a> + 'a>(
Expand All @@ -43,20 +43,20 @@ impl<'a> Table<'a> {
if o == 0 {
return default;
}
Some(<T>::follow(self.buf, self.loc + o))
Some(unsafe { <T>::follow(self.buf, self.loc + o) })
}
}

impl<'a> Follow<'a> for Table<'a> {
type Inner = Table<'a>;
#[inline]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
unsafe fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
Table { buf, loc }
}
}

#[inline]
pub fn buffer_has_identifier(data: &[u8], ident: &str, size_prefixed: bool) -> bool {
pub unsafe fn buffer_has_identifier(data: &[u8], ident: &str, size_prefixed: bool) -> bool {
assert_eq!(ident.len(), FILE_IDENTIFIER_LENGTH);

let got = if size_prefixed {
Expand Down