Okay, there's how I see it.

First goes the trait:

/// A trait to access untrusted data in the flatbuffer.
///
/// # Safety
///
/// * results should be stable (pure):
///
///     * for all calls with the same `buf` and `loc`
///         `follow` *must* return the same results.
///
/// * the trait implementation should guarantee that if `follow` returned `Some`
///     then call to `follow_unchecked` with the same `buf` and `loc`
///     would not invoke UB.
pub unsafe trait FollowChecked<'a>
where
    Self: Follow<'a>,
{
    /// Checked getter for untrusted data.  
    ///
    /// # Note to implementors
    ///
    /// Please, try to avoid panics. 
    fn follow(buf: &'a [u8], loc: usize) -> Option<Self::Inner>;
}

The name of the function is a bikeshed question.

Let's look at how it would be implemented for u64:

unsafe impl FollowChecked<'_> for u64 {
    #[inline]
    fn follow(buf: &[u8], loc: usize) -> Option<Self::Inner> {
        buf.get(loc..loc + size_of::<Self>()).and_then(|s| {
            from_raw_bytes(s).map(|x| x.to_le())
        })
    }
}

/// Transmutation function.
#[inline]
pub fn from_raw_bytes<T: FromRawBytes>(s: &[u8]) -> Option<&T> {
    assert!(size_of::<T>() != 0);

    let ptr = s.as_ptr();
    if ptr as usize % align_of::<T>() == 0 && s.len() >= size_of::<T> {
        // SAFETY:
        // there is at least `sizeof(T)` bytes,
        // and the reference is aligned;
        // transmute safety should be guaranteed by trait implementor.
        Some(unsafe { &*(ptr as *const T) })
    } else {
        None
    }
}

/// Marker trait for types that could be safely read from raw aligned bytes
/// in a generic way.
///          
/// # Safety
///
/// * implementor type *must* be non-zero sized;
///                                                                       
/// * it *must* be safe and sound to transmute
///
///     * `[u8; size_of::<Self>()]` into `Self`,
///
///     * `[u8; N * size_of::<Self>()]` into `[Self; N]`,
///
/// # Note
///
/// It is not the same as [`AsRawBytes`]! For example, \
/// `#[repr(C)] struct Q(u8, u16);` \
/// can implement `FromRawBytes`, but can't implement `AsRawBytes`,
/// because implicit padding is treated as uninitialized memory in Rust!   
pub unsafe trait FromRawBytes: Copy {}

// SAFETY:
// all scalars could be safely transmuted from and into raw bytes.
unsafe impl FromRawBytes for u64 {}

There are things like enums and bitflag-enums, for which we may want to check values or may not want.
If we go #6098 way and do not check enum and bitflag values while parsing the data (without adding #[repr(transparent)] to those newtype enums and bitflag structs it is UB to read them from bytes, pls fix, @CasperN !), then we don't check anything in structs besides alignment and size, because any aligned slice of initialized bytes would be valid for the struct if it has length big enough.
If we go with native enums like #6158 , we'd need to check enums and structs that contain enums or contain structs with enums (recursion limit reached).

Follow trait, tables and slices (in flatbuffer you call it "vectors", but in Rust vectors are resizable, and those slices are not resizable, nor are they constant-sized to be called "array"), would need rework to be sound and to use safety guarantees we can provide with FollowChecked.

Let's start with tables:

#[derive(Debug)]
pub struct Table<'a> {
    buf_start: NonNull<u8>,
    loc: usize,
    _lifetime: PhantomData<&'a [u8]>,
}

impl<'a> Table<'a> {
    #[doc(hidden)]
    #[inline]
    pub const fn new(buf_start: NonNull<u8>, loc: usize) -> Self {
        Self {
            buf_start,
            loc,
            _lifetime: PhantomData,
        }
    }

    /// # Safety
    ///
    /// Absolutely none. It should not be ever called anywhere but autogenerated code.
    #[inline]
    unsafe fn vtable(&self) -> &'a [VOffset] {
        let buf_start = self.buf_start.get();
        let offset = SOffset::follow_unchecked(buf_start, self.loc);
        let vtable_loc = (self.loc as isize - offset as isize) as usize;
        let vtable_start = buf_start.add(vtable_loc);
        let vtable_len = VOffset::follow_unchecked(vtable_start, 0);
        slice::from_raw_parts(vtable_start as *const VOffset, vtable_len as usize / size_of::<VOffset>())
    }

    /// # Safety
    ///
    /// Absolutely none. It should not be ever called anywhere but autogenerated code.
    #[inline]
    pub unsafe fn get_optional<T>(&self, vtable_slot: usize) -> Option<T::Inner>
    where
        T: ?Sized + Follow<'a>,
    {
        unsafe { self.vtable() }
            .get(vtable_slot)
            .and_then(|&table_offset| {
                if table_offset == 0 {
                    None
                } else {
                    // SAFETY:
                    // should be guaranteed to be safe by the caller.
                    Some(unsafe { T::follow_unchecked(self.buf_start.get(), self.loc + table_offset as usize) })
                }
            })
    }

    /// # Safety
    ///
    /// Absolutely none. It should not be ever called anywhere but autogenerated code.
    #[inline]
    pub unsafe fn get_required<T>(&self, vtable_slot: VOffset) -> T::Inner
    where
        T: ?Sized + Follow<'a>,
    {
        debug_assert!(self.vtable().len() > vtable_slot as usize);
        // SAFETY:
        // should be guaranteed to be safe by the caller.
        unsafe {
            T::follow_unchecked(
                self.buf_start.get(),
                self.loc + *self.vtable().get_unchecked(vtable_slot as usize) as usize,
            )
        }
    }
}

Yeah, tables are unsafe as hell. How do we implement FollowChecked for tables then? We don't. Instead, we use this macro to implement FollowChecked for each wrapper struct around table:

/// Given a `buf`, `loc` and a list of fields
///
/// * for unions:
///     `{ (UnionType, is_required, union_vtable_slot, associated_ptr_vtable_slot)
///         match { [union_value => AssociatedType,] }`
///
/// * for any other type:
///     `(Type, is_required, vtable_slot)`
///
/// returns `Some(Table)` if all fields given could be followed,
/// `None` otherwise.
///
/// Note: `*_slot` must be an offset in `VOffset`s from the start of vtable,
/// not in bytes! It also must be of type `usize`.
///
/// Another note: types must be explicitly required!
/// Unlike the schema, this macro allows structs and scalars to be optional.
#[macro_export]
macro_rules! follow_table {
    ($buf:expr, $loc:expr, [ $( $field:tt, )* ]) => {{
        #[inline]
        fn table_follow<'a>(buf: &'a [u8], loc: usize) -> Option<$crate::Table<'a>> {
            use $crate::{
                core::{mem::{size_of, align_of}, slice, ptr::NonNull}, SOffset, VOffset, FollowChecked,
            };

            match SOffset::follow(buf, loc) {
                None => None,
                Some(offset) => {
                    // `loc` could only have value in `0..isize::MAX` range
                    // due to slice restrictions (already successfully indexed the slice with `loc`).
                    // For any possible `loc`, whenever it overflows, it would stay in
                    // negative `isize` range, thus would not be a valid index into `buf`;
                    // whenever it underflows, it also would stay in negative `isize` range;
                    // thus this would never return `Some` on underflow or overflow,
                    // thus it is indeed stable, and if this returned `Some`,
                    // it would not be UB to call `follow_unchecked` with the same `buf` and `loc`
                    // (if all table types are checked, of course).
                    let vtable_loc = (loc as isize - offset as isize) as usize;
                    let vtable = buf.get(vtable_loc..)?;

                    let vtable: &'a [VOffset] = {
                        // This `follow` checks that `vtable` is aligned for `VOffset`.
                        let vtable_len = VOffset::follow(vtable, 0)? as usize;
                        debug_assert_eq!(vtable.as_ptr() as usize % align_of::<VOffset>(), 0);

                        let v = buf.get(vtable_loc..vtable_loc + vtable_len)?;
                        // SAFETY:
                        // it is aligned for `VOffset`,
                        // and `VOffset` could be safely transmuted from bytes.
                        unsafe { slice::from_raw_parts(v.as_ptr() as *const VOffset, v.len() / size_of::<VOffset>()) }
                    };

                    $(
                        $crate::follow_table! { vtable, buf, loc, $field }
                    )*

                    Some($crate::Table::new(NonNull::from(buf.first().unwrap()), loc))
                }
            }
        }

        table_follow($buf, $loc)
    }};
    // Union branch.
    ($vtable:expr, $buf:expr, $loc:expr,
        {
            ($union_type:ty, $req:expr, $vtable_union_slot:expr, $vtable_assoc_type_slot:expr)
            match {$(
                $union_value:expr => $associated_type:ty,
            )*}
        }
    ) => {{
        let (vtable, buf, loc) = ($vtable, $buf, $loc);
        if $req {
            let union_table_offset = *vtable.get($vtable_union_slot)?;
            if union_table_offset == 0 {
                // Required can't be default.
                return None;
            } else {
                let assoc_loc = loc + *vtable.get($vtable_assoc_type_slot)? as usize;
                match <$union_type>::follow(buf, loc + union_table_offset as usize)? {
                    $(
                        $union_value => { <$associated_type>::follow(buf, assoc_loc)?; },
                    )*
                    // If union is `None` or unknown value, it is always valid, yet ignored.
                    _ => (),
                }
            }
        } else {
            // If the slot is not required and is out of vtable range,
            // then it is always valid to get, but default.
            if let Some(&union_table_offset) = vtable.get($vtable_union_slot) {
                // If it is `0`, then it is default, then it is valid.
                if union_table_offset != 0 {
                    let assoc_loc = loc + *vtable.get($vtable_assoc_type_slot)? as usize;
                    match <$union_type>::follow(buf, loc + union_table_offset as usize)? {
                        $(
                            $union_value => { <$associated_type>::follow(buf, assoc_loc)?; },
                        )*
                        // If union is `None` or unknown value, it is always valid, yet ignored.
                        _ => (),
                    }
                }
            }
        }
    }};
    // Any other field branch.
    ($vtable:expr, $buf:expr, $loc:expr, ($t:ty, $req:expr, $vtable_slot:expr)) => {{
        if $req {
            let table_offset = *$vtable.get($vtable_slot)?;
            if table_offset == 0 {
                // Required can't be default.
                return None;
            } else {
                <$t>::follow($buf, $loc + table_offset as usize)?;
            }
        } else {
            // If the slot is not required and is out of vtable range,
            // then it is always valid to get, but default.
            if let Some(&table_offset) = $vtable.get($vtable_slot) {
                // If it is `0`, then it is default, then it is valid.
                if table_offset != 0 {
                    <$t>::follow($buf, $loc + table_offset as usize)?;
                }
            }
        }
    }};
}

Because we know all three parts of the triplet this macro needs at schema compilation time, this macro is easy to use in the generated code. Though, this macro could generate a lot of Rust code, and to speed up unchecked compilations we may want to put FollowChecked implementations into another file, so it would be linked only when it is really needed.

The macro is used like this:

#[cfg(test)]
mod tests {
    /// Welp, yeah, tests that this thing compiles at all.
    #[test]
    fn it_works() {
        assert!(follow_table! {
            &[],
            0,
            [
                // Union field syntax.
                { (u64, true, 4, 5)
                    match {
                        // Actual types would be `ForwardsUOffset<str/[u8]>`;
                        // anyway, this is enough for a compilation test.
                        1 => str,
                        2 => [u8],
                    }
                },
                // Any other type field syntax.
                (u32, true, 2),
                (u16, true, 3),
            ]
        }
        .is_none());
    }
}

Slices would look like table: pointer to the buffer start, location and lifetime phantom marker.
FollowChecked should be implemented like this:

// Option<Self::Inner>
fn follow(buf: &'a [u8], loc: usize) -> Option<Slice<'a, T>> {
    UOffset::follow(buf, loc).and_then(|len| {
        let len = len as usize * size_of::<T>();
        let start = loc + size_of::<UOffset>();
        buf.get(start..start + len).and_then(|_| {
            let mut ptr = start;
            while ptr < start + len {
                T::follow(buf, ptr)?;
                ptr += size_of::<T>();
            }
        })
        Some(unsafe { Slice { buf_start: NonNull::from(buf.first().unwrap()), loc: loc, _lifetime: PhantomData })
    })
}

About the Follow trait: it doesn't actually need a [u8] for trusted access, only *const u8 that points to the start of the buffer.
And to allow tables and slices be only two usize wide, it should look like this:

/// A trait to access trusted data in the flatbuffer.
///
/// # Safety
///
/// Absolutely none. Implementing this trait outside this lib or autogenerated code is UB.
///
/// Consider this trait an implementation detail,
/// that we cannot hide due to autogenerated code.
///
/// ## But I'm writing for the lib!
///
/// Welp, then
///
/// * if `Self` is sized, then `sizeof(Self)` must equal to the actual number of bytes your implementation would read,
///     and `Self` must be non-zero-sized type;
///
/// * results should be stable (pure):
///
///     * for all calls with the same `buf` and `loc
///         `follow_unchecked` *must* return the same results
///         if the call with the given `buf` and `loc` is not UB.
pub unsafe trait Follow<'a> {
    /// The target type to aquire.
    type Inner;

    /// Unchecked getter for already checked or trusted data.
    ///
    /// # Safety
    ///
    /// Absolutely none. Using this function outside of the lib is UB.
    ///
    /// ## But I'm writing for the lib!
    ///
    /// Read inexhaustive list of when `FollowChecked::follow` returns `None`.
    /// Calling this function when `buf.wrapping_add(loc)` is unaligned
    /// or when `follow` returns `None` is UB.
    unsafe fn follow_unchecked(buf_start: *const u8, loc: usize) -> Self::Inner;
}

The list:

/// # When it returns `None`
///
/// First: if it always returns `None` whenever `&buf[loc]` is unaligned.
/// When is it unaligned? Well… It depends on the root type mostly,
/// but `Follow` is used to follow any type, not just root,
/// thus you can't actually check whether the given `buf` and `loc`
/// would produce aligned access or unaligned.
///
/// For undocumented implementations' behaviour, read the implementation description.
///
/// ## Primitives
///
/// For `u8, i8, u16, i16, u32, i32, u64, i64, f32, f64`
/// it returns `None` if `buf.len() < loc + size_of::<T>()` where `T` is one of those types.
///
/// ## Booleans
///
/// For [`bool`] and [`Bool`] it returns `None`
/// whenever `u8::follow(buf, loc)` returns `None`.
///
/// ## Native Rust slices
///
/// For `[T]` it returns `None` when
///
/// * `UOffset::follow(buf, loc)` returns `None`;
///
/// * `buf.len()
///     < loc
///         + size_of::<UOffset>()
///         + UOffset::follow(buf, loc).unwrap() as usize * size_of::<T>()`
///
/// Just to clarify things: [`UOffset`] is a type alias for one of primitives.
///
/// ## Flatbuffer slices
///
/// Same as native slices, plus it returns `None` if following any of the items in the slice returned `None`.
///
/// For more info read [`Slice::new`] documentation.
///
/// ## Strings
///
/// For string slices ([`str`]) it returns `None` when
///
/// * `<[u8]>::follow(buf, loc)` returns `None`;
///
/// * result of `<[u8]>::follow(buf, loc).unwrap()` does not contain valid UTF-8 sequence.
///
/// ## Offsets
///
/// For [`ForwardsUOffset<T>`] it returns `None` when
///
/// * `UOffset::follow(buf, loc)` returns `None`;
///
/// * `T::follow(buf, loc + UOffset::follow(buf, loc).unwrap() as usize)`
///     returns `None`.
///
/// [`bool`]: https://doc.rust-lang.org/std/primitive.bool.html
/// [`Bool`]: struct.Bool.html
/// [`UOffset`]: type.UOffset.html
/// [`Slice::new`]: slice/struct.Slice.html#method.new_unchecked
/// [`str`]: https://doc.rust-lang.org/std/primitive.str.html
/// [`ForwardsUOffset<T>`]: struct.ForwardsUOffset.html

The special flatbuffers Bool that allows to get a slice of bools from flatbuffer slice:

/// A special flatbuffers bool.
///
/// Flatbuffers booleans are simple: any `u8` value is valid for it;
/// if the inner `u8` equal zero, then `Bool` maps to `false`,
/// any other value maps to `true`.
#[derive(Copy, Clone)]
#[repr(transparent)]
pub struct Bool(u8);

// SAFETY:
// `repr(transparent)` over `u8`;
// could be safely cast to bytes.
unsafe impl AsRawBytes for Bool {}

// `Bool` is one byte wide, always ordered.
impl ByteOrdered for Bool {}

impl Bool {
    /// Constructor function.
    #[inline]
    pub const fn new(u: u8) -> Self {
        Self(u)
    }

    /// Casts this custom `Bool` into the real Rust `bool`.
    #[inline]
    pub const fn as_bool(self) -> bool {
        self.0 != 0
    }

    /// Casts a `&u8` to `&Bool`.
    #[inline]
    pub fn from_ref(u: &u8) -> &Self {
        // SAFETY: `repr(transparent)`, thus it's safe
        unsafe { &*(u as *const u8 as *const Bool) }
    }

    /// Casts a slice of bytes to slice of `Bool`s.
    #[inline]
    pub fn from_slice(s: &[u8]) -> &[Bool] {
        // SAFETY:
        // `repr(transparent)` over `u8` guarantee safe transmutation,
        // while other invariants are guaranteed by `s` being a valid slice
        // of items with the same align and size as `Bool`.
        unsafe { slice::from_raw_parts(s.as_ptr() as *const Bool, s.len()) }
    }
}

Marker traits and transmutation functions that would replace unsound EndianScalar, emplace_scalar and read_scalar:

use core::{
    mem::{align_of, size_of},
    slice,
};

/// Transmutation function.
#[inline]
pub fn as_raw_bytes<T: AsRawBytes>(t: &T) -> &[u8] {
    assert!(size_of::<T>() != 0);

    // SAFETY:
    // transmute safety should be guaranteed by trait implementor.
    unsafe { slice::from_raw_parts(t as *const T as *const u8, size_of::<T>()) }
}

/// Slice transmutation function.
#[inline]
pub fn slice_as_raw_bytes<T: AsRawBytes>(s: &[T]) -> &[u8] {
    assert!(size_of::<T>() != 0);

    // SAFETY:
    // the pointer and length are obtained from valid slice, thus pointer is valid,
    // and slice length *in bytes* is never bigger than `isize::MAX`;
    // transmute safety should be guaranteed by trait implementor.
    unsafe { slice::from_raw_parts(s.as_ptr() as *const u8, s.len() * size_of::<T>()) }
}

/// Transmutation function.
#[inline]
pub fn from_raw_bytes<T: FromRawBytes>(s: &[u8]) -> Option<&T> {
    assert!(size_of::<T>() != 0);

    let ptr = s.as_ptr();
    if ptr as usize % align_of::<T>() == 0 && s.len() >= size_of::<T> {
        // SAFETY:
        // there is at least `sizeof(T)` bytes,
        // and the reference is aligned;
        // transmute safety should be guaranteed by trait implementor.
        Some(unsafe { &*(ptr as *const T) })
    } else {
        None
    }
}

/// Slice transmutation function.
#[inline]
pub fn slice_from_raw_bytes<T: FromRawBytes>(s: &[u8]) -> Option<&[T]> {
    assert!(size_of::<T>() != 0);

    let ptr = s.as_ptr();
    if ptr as usize % align_of::<T>() == 0 {
        // SAFETY:
        // the pointer and length are obtained from valid slice,
        // and pointer is checked to be aligned for `T`;
        // transmute safety should be guaranteed by trait implementor.
        Some(unsafe { slice::from_raw_parts(ptr as *const T, s.len() / size_of::<T>()) })
    } else {
        None
    }
}

/// Slice transmutation function that does not check alignment.
///
/// # Safety
///
/// `s` must be aligned for `T`.
#[inline]
pub unsafe fn slice_from_raw_bytes_unchecked<T: FromRawBytes>(s: &[u8]) -> &[T] {
    assert!(size_of::<T>() != 0);

    // SAFETY:
    // the pointer and length are obtained from valid slice,
    // and pointer is checked to be aligned for `T`;
    // transmute safety should be guaranteed by trait implementor.
    unsafe { slice::from_raw_parts(s.as_ptr() as *const T, s.len() / size_of::<T>()) }
}

/// Marker trait for types that are represented in little endian byte order.
pub trait ByteOrdered {}

/// Marker trait for types that could be safely represented as raw bytes
/// in a generic way.
///
/// # Safety
///
/// * implementor type *must* be non-zero sized;
///
/// * it *must* be safe and sound to transmute
///
///     * `Self` into `[u8; size_of::<Self>()]`;
///
///     * `[Self; N]` into `[u8; N * size_of::<Self>()]`,
///
/// # Note
///
/// It is not the same as [`FromRawBytes`]! For example, `NonZeroI32`
/// can implement `AsRawBytes`, but can't implement `FromRawBytes`,
/// because raw bytes could contain invalid value for `NonZeroI32`!
pub unsafe trait AsRawBytes: Copy {}

/// Marker trait for types that could be safely read from raw aligned bytes
/// in a generic way.
///
/// # Safety
///
/// * implementor type *must* be non-zero sized;
///
/// * it *must* be safe and sound to transmute
///
///     * `[u8; size_of::<Self>()]` into `Self`,
///
///     * `[u8; N * size_of::<Self>()]` into `[Self; N]`,
///
/// # Note
///
/// It is not the same as [`AsRawBytes`]! For example, \
/// `#[repr(C)] struct Q(u8, u16);` \
/// can implement `FromRawBytes`, but can't implement `AsRawBytes`,
/// because implicit padding is treated as uninitialized memory in Rust!
pub unsafe trait FromRawBytes: Copy {}

The name SafeSliceAccess or ByteOrdered is a bikeshed, but I don't SafeSliceAccess, because it has nothing to do with safetyness, only markers endianness and could be safely implemented unlike FromRawBytes and AsRawBytes.

Though, AsRawBytes, its transmutation functions (which we don't really need, because builder should be aligned), and changes to make Push trait sound should be a part of another issue or PR.

Implementing codegen for this would be easier with #6098 , because recursive struct checks are a little bit tricky.

Any questions other than bikeshedding follow[_checked] and follow[_unchecked]?

Edit: forgot to tell about unbounded lifetime in the follow_unchecked — it is not a problem, because we bound it in the get_root_unchecked:

pub unsafe fn get_root_unchecked<'a, T>(buf: &'a [u8]) -> T
where
    T: Follow<'a> + Root
{
    debug_assert_eq!(buf.as_ptr() as usize % align_of::<T::Aligned>(), 0);
    ForwardsOffset<T>::follow_unchecked(buf.as_ptr(), 0)
}

Edit 2: okay, forgot about Root trait:

/// # Safety
///
/// Absolutely none. Implementing it anywhere but generated code is UB.
///
/// ## But I'm writing the codegen!
///
/// Then `Aligned` must be a generated type that looks like
/// `#[repr(C, align(N))] pub struct BytesFor[RootIdent]([u8; N]);`
/// where `N` is the maximum possible alignment for the root
/// (which is the maximujm of all possibly-contained structs, including nested flatbuffers),
/// and `[RootIdent]` is the name of the root.
pub unsafe trait Root {
    type Aligned: FromRawBytes + AsRawBytes;
}

This trait would also allow provide some safety guarantees for some of the builder buffers, because
assert!(align_of::<T>() <= align_of::<Root::Aligned>());
would be evaluated at constant time: it either never panics or always panics for T we push, and compiler can optimize this to no assert for things that never panic.

Without this trait creating builder would always be unsafe. Though, it's not yet clear how builder would look to use the Root information in a safe way, and it is not about verifier anyway.

Implement Flatbuffer Verifier for Rust #6161

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions