Implement Flatbuffer Verifier for Rust

[CasperN]

Tracking issue for the title.

@rw @krojew @ImmemorConsultrixContrarie

[rw]

cc @aardappel

[krojew]

Linking cfb verifier for reference: https://github.com/nervosnetwork/cfb

[ImmemorConsultrixContrarie]

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.

[krojew]

The biggest problem with this idea (besides a ton of unsafe) is that it continues to work on raw u8 slices, which doesn't give us anything as far as validation is concerned. The verifier should return a type representing a validated view into a given buffer (this was discussed some time ago). Therefore we would get essentially two ways of accessing data:

• One with untrusted access, which would work similarly to now. Checked access with Results/Options/panics. Unchecked can be provided where applicable, when the user trusts the buffer implicitly.
• One with trusted access on validated view. This one can skip all checks and retrieve the data the fastest way possible, including wrapping unsafe transmutations in safe functions, if needed.

Our use cases can be broken down as:

1. Implicitly trusted buffer with fast access.
2. Untrusted buffer with checked access.
3. Validated buffer with fast access.

Number 1 can be achieved with unchecked access to a slice and is inherently unsafe. The user takes full responsibility for trusting the buffer.
2 is, more or less, what we have now. The difference being, we should return Results/Options where applicable rather than a hard panic. Lack of this makes current Rust API unusable on production with untrusted buffers, unless someone uses cfb verification.
Number 3 can be achieved by a trusted view into the data, as explain above. Am I missing something @rw @CasperN @aardappel ?

[ImmemorConsultrixContrarie]

The biggest problem with this idea (besides a ton of unsafe) is that it continues to work on raw u8 slices, which doesn't give us anything as far as validation is concerned. The verifier should return a type representing a validated view into a given buffer (this was discussed some time ago). Therefore we would get essentially two ways of accessing data:

* One with untrusted access, which would work similarly to now. Checked access with Results/Options/panics. Unchecked can be provided where applicable, when the user trusts the buffer implicitly.

* One with trusted access on validated view. This one can skip all checks and retrieve the data the fastest way possible, including wrapping unsafe transmutations in safe functions, if needed.

Our use cases can be broken down as:

1. Implicitly trusted buffer with fast access.

2. Untrusted buffer with checked access.

3. Validated buffer with fast access.

Number 1 can be achieved with unchecked access to a slice and is inherently unsafe. The user takes full responsibility for trusting the buffer.
2 is, more or less, what we have now. The difference being, we should return Results/Options where applicable rather than a hard panic. Lack of this makes current Rust API unusable on production with untrusted buffers, unless someone uses cfb verification.
Number 3 can be achieved by a trusted view into the data, as explain above. Am I missing something @rw @CasperN @aardappel ?

Yes. This FollowChecked::follow would return either a validated buffer with safe and fast access or None; all getters in structs are safe, even though they use a lot of unsafety inside. The generated files should start from a comment about unsafe tainting, yeah, forgot that one. Your number one is what get_root_unchecked would return. Number two is something that would not be used often: the buffer is either valid/trusted, or it is invalid, isn't it?

And the problem is that we don't have anything of this right now. The getters are not the fastest, nor are they sound and totally safe. We cannot actually parse flatbuffer into valid struct without unsafe inside, because that would involve mutation of the buffer (which cannot be done for a number of reasons) and/or new allocations. However, we can totally validate it and guarantee that if user only uses safe functions from the generated code and no raw constructors, it is safe; and that's what FollowChecked does.

Getters in tables would look like

fn get_color(&self) -> Color {
    unsafe { self.tab.get_required(Self::COLOR_SLOT) }
}

But for get_root_unchecked we lift that unsafe into get_root_unchecked itself, and for safe get_root that could return None we validate that this is safe when we follow the table.

Edit: "ton of unsafe" is not a problem, it is reality. All those things that are unsafe are unsafe, and no less.

[krojew]

2 is useful when interested only in a subset of data. So that FollowChecked would be implemented for every field or just for the table? It looks like for every field, which defeats the purpose of one-time validation. Also, what's the definition of follow_unchecked?

[ImmemorConsultrixContrarie]

Can C/C++ verifier check only subset of data in the schema?

Anyway, you can do this (even if not with how flatc works right now) by shrinking the schema down to fields you are interested in. Flatc won't allow you to write table Human { name: string (id: 8); }, but if it would, it'd be easy to generate getters only for a required subset.

Edit: and if we'd use newtype enums instead of native, we won't ever need to follow struct fields, simplifying struct following to a one from_raw_bytes call.

[krojew]

Can C/C++ verifier check only subset of data in the schema?

Irrelevant for the use case.

Anyway, you can do this (even if not with how flatc works right now) by shrinking the schema down to fields you are interested in. Flatc won't allow you to write table Human { name: string (id: 8); }, but if it would, it'd be easy to generate getters only for a required subset.

Also - irrelevant. This is a use case which we can support with low effort. What's relevant are my two previous questions. Can you answer them, so we can see if this is a viable way?

[ImmemorConsultrixContrarie]

You wanna answers? Nice.

Also, what's the definition of follow_unchecked?

I wrote. Re-read my first comment, it is there.

So that FollowChecked would be implemented for every field or just for the table

No, it can't. But can you implement something like table with all getters both safe and as fast as possible? FollowChecked is a hit at two ways that flatbuffers are used most: totally trusted or totally untrusted accesses.

For example, if you worked with trusted buffers, but then started to work with untrusted (for example, when it was local first, and then also network), with FollowChecked the only thing you have to rewrite is one place where you get the root.

When all getters are either unsafe or safe but fallible, you need to rewrite all the code that works with it. It is against DRY, it is against "parse, don't validate", and finally, it is uncommon use case, which we can easily make working with a little change in schema parser.

[ImmemorConsultrixContrarie]

Found a problem in the macro for tables: can't check unions. There actually should be a match for a union, and only then we'd know types. Shouldn't be too hard to fix, though.

[krojew]

I searched for the definition of follow_unchecked and there seems to be none, hence my question. Also from your second answer I can't tell what "no" refers to. You mean it will be implemented for each field or the whole table once?

[ImmemorConsultrixContrarie]

I searched for the definition of follow_unchecked and there seems to be none, hence my question. Also from your second answer I can't tell what "no" refers to. You mean it will be implemented for each field or the whole table once?

Only the whole table. If you check the table partially, but generate getters for all fields, it would be unsound. Unless, of course, you do not generate double getter "unsafe / safe-fallible" for unchecked fields.

I think, you should read flatCC verification doc.

I mostly followed this, with a bunch of exceptions, that could be closed lately, if needed:
Rust str is always UTF-8 and not zero-terminated;
table macro does not check table length, only that all slots are accessible;
vtable could be of any size, even zero, it only has to be aligned (because we don't check table length) — if table has any required field, then for such vtable we would return None, and if no required fields, then all fields are default;
UOffset does not need to have value at least sizeof(UOffset) — the pointee only has to be accessible and generated pointer has to be aligned;
FollowChecked has no recursion limit. This could be a problem, actually, but changing function to follow(buf: &[u8], loc: usize, recursion_levels_left: usize) should fix it.

tl;dr: FollowChecked catches only memory unsafety invariants, and none logical bugs.

Edit: follow_unchecked definition is a part of my first comment. Really.

Image

[krojew]

You provided a description, not definition. Please provide a definition, so we can see the whole proposed process of retrieving data.

[ImmemorConsultrixContrarie]

You provided a description, not definition. Please provide a definition, so we can see the whole proposed process of retrieving data.

That's how it looks for u64 (and any other integer/float (actually, floats would use from_bits(to_bits().to_le())):

unsafe fn follow_unchecked(buf_start: *const u8, loc: usize) -> u64 {
    // SAFETY: this is guaranteed to be safe by the caller.
    unsafe { (*(buf_start.add(loc) as *const u64)).to_le() }
}

For tables it simply stores buf_start and loc in the table. Same for slices. For structs it would look like this:

unsafe fn follow_unchecked(buf_start: *const u8, loc: usize) -> &'a Struct {
    // SAFETY: this is guaranteed to be safe by the caller.
    unsafe { &*(buf_start.add(loc) as *const Struct) }
}

If enums are just a repr(transparent) newtype around integer — NewtypeEnum::new(ReprInt::follow_unchecked()).

For native enums it would be a transmute after following integer. But we can't transmute, lol, because it could be invalid value and UB; invalid values are not forbidden by flatbuffer schema evolution strategy. Thus only newtype enums could be followed, and native enums could be only stored as integer.
Thus newtype enums win, because they could be stored as a real type, not only as integer. And if you need, they also could be converted to native Rust enums, just like integer, using exactly same logic.
Really, it's not just about this new Follow, it's also about current Follow too, lol.

Are there any other types I forgot about?