Skip to content

[Rust] Verify nested_flatbuffer fields #9230

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.

Sign up for GitHub

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

Open
sanil18 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Rust] Verify nested_flatbuffer fields #9230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rust/flatbuffers/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ pub use crate::push::{Push, PushAlignment};
pub use crate::table::{buffer_has_identifier, Table};
pub use crate::vector::{follow_cast_ref, Vector, VectorIter};
pub use crate::verifier::{
ErrorTraceDetail, InvalidFlatbuffer, SimpleToVerifyInSlice, TableVerifier, Verifiable,
Verifier, VerifierOptions,
ErrorTraceDetail, InvalidFlatbuffer, NestedFlatBuffer, SimpleToVerifyInSlice, TableVerifier,
Verifiable, Verifier, VerifierOptions,
};
pub use crate::vtable::field_index_to_field_offset;
pub use bitflags;
Expand Down
67 changes: 66 additions & 1 deletion rust/flatbuffers/src/verifier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::follow::Follow;
use crate::{ForwardsUOffset, SOffsetT, SkipSizePrefix, UOffsetT, VOffsetT, Vector, SIZE_UOFFSET};
#[cfg(not(feature = "std"))]
use alloc::vec::Vec;
use core::marker::PhantomData;
use core::ops::Range;
use core::option::Option;

Expand Down Expand Up @@ -242,9 +243,17 @@ pub struct VerifierOptions {
/// Ignore errors where a string is missing its null terminator.
/// This is mostly a problem if the message will be sent to a client using old c-strings.
pub ignore_missing_null_terminator: bool,
/// Verify the contents of fields carrying the `nested_flatbuffer` schema
/// attribute, rather than only checking that the byte vector holding them is
/// in bounds.
///
/// This defaults to `true`, matching `check_nested_flatbuffers` in the C++
/// implementation. Turning it off makes the generated
/// `..._nested_flatbuffer()` accessors unsound for untrusted input, because
/// they follow those bytes without any further checking.
pub check_nested_flatbuffers: bool,
// probably want an option to ignore utf8 errors since strings come from c++
// options to error un-recognized enums and unions? possible footgun.
// Ignore nested flatbuffers, etc?
}

impl Default for VerifierOptions {
Expand All @@ -255,6 +264,7 @@ impl Default for VerifierOptions {
// size_ might do something different.
max_apparent_size: 1 << 31,
ignore_missing_null_terminator: false,
check_nested_flatbuffers: true,
}
}
}
Expand Down Expand Up @@ -388,6 +398,35 @@ impl<'opts, 'buf> Verifier<'opts, 'buf> {
Ok(TableVerifier { pos: table_pos, vtable: vtable_pos, vtable_len, verifier: self })
}

/// Verifies the contents of a nested FlatBuffer: the bytes of a `[ubyte]`
/// field carrying the `nested_flatbuffer` schema attribute, whose root type
/// is `T`.
fn verify_nested_buffer<T: Verifiable>(&mut self, range: Range<usize>) -> Result<()> {
// `range` was produced by `verify_vector_range`, which has already
// bounds checked it. It is re-checked here rather than indexed because
// the verifier must not panic on any input, however malformed, and a
// future caller of this helper should not be able to turn a mistake into
// a panic.
let nested = match self.buffer.get(range.clone()) {
Some(nested) => nested,
None => return InvalidFlatbuffer::new_range_oob(range.start, range.end),
};

// Offsets inside the nested buffer are relative to its own start, so the
// verifier has to run against the nested slice rather than adjust a
// position. The buffer is swapped in place instead of building a second
// `Verifier` so that every budget -- `depth`, `num_tables`,
// `apparent_size`, and any added later -- keeps accumulating in `self`.
// A chain of nested buffers therefore cannot escape `max_tables` or
// `max_apparent_size` by starting each level from zero, and the accounting
// cannot silently drift if a new budget field is added, since there is no
// per-field copy to keep in sync.
let outer = core::mem::replace(&mut self.buffer, nested);
let res = <ForwardsUOffset<T>>::run_verifier(self, 0);
self.buffer = outer;
res
}

/// Runs the union variant's type's verifier assuming the variant is at the given position,
/// tracing the error.
pub fn verify_union_variant<T: Verifiable>(
Expand Down Expand Up @@ -563,6 +602,32 @@ impl<T: SimpleToVerifyInSlice> Verifiable for Vector<'_, T> {
}
}

/// Verification marker for the `nested_flatbuffer` schema attribute: a `[ubyte]`
/// field whose contents are themselves a FlatBuffer with root type `T`.
///
/// The generated `..._nested_flatbuffer()` accessor follows those bytes without
/// any further bounds checking, so verifying the field only as `Vector<u8>`
/// leaves that accessor reading unverified data. Verifying it as
/// `NestedFlatBuffer<T>` checks the byte vector and then the buffer inside it,
/// mirroring `VerifyNestedFlatBuffer` in the C++ implementation.
///
/// Unlike the other `Verifiable` types this deliberately does not implement
/// `Follow`: the accessor reads the nested buffer through `ForwardsUOffset<T>`,
/// so this marker exists only to carry the extra verification and can never be
/// used to read data.
pub struct NestedFlatBuffer<T>(PhantomData<T>);

impl<T: Verifiable> Verifiable for NestedFlatBuffer<T> {
#[inline]
fn run_verifier(v: &mut Verifier, pos: usize) -> Result<()> {
let range = verify_vector_range::<u8>(v, pos)?;
if !v.opts.check_nested_flatbuffers {
return Ok(());
}
v.verify_nested_buffer::<T>(range)
}
}

impl<T: Verifiable> Verifiable for SkipSizePrefix<T> {
#[inline]
fn run_verifier(v: &mut Verifier, pos: usize) -> Result<()> {
Expand Down
16 changes: 16 additions & 0 deletions src/idl_gen_rust.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,22 @@ class RustGenerator : public BaseGenerator {
if (GetFullType(field.value.type) != ftUnionValue) {
// All types besides unions.
code_.SetValue("TY", FollowType(field.value.type, "'_"));
// A field with the `nested_flatbuffer` attribute is a [ubyte] vector
// whose contents are themselves a flatbuffer. The generated
// `..._nested_flatbuffer()` accessor follows those bytes without any
// further bounds checking, so the bytes have to be verified as a buffer
// and not merely as a vector. This mirrors VerifyNestedFlatBuffer in
// the C++ generator.
//
// The root type is resolved by the parser, which errors out if it was
// never defined, so there is no lookup here that could fail and quietly
// leave the field verified as a plain vector.
if (field.nested_flatbuffer) {
code_.SetValue("TY",
"::flatbuffers::ForwardsUOffset<"
"::flatbuffers::NestedFlatBuffer<" +
WrapInNameSpace(*field.nested_flatbuffer) + ">>");
}
code_ +=
" .visit_field::<{{TY}}>(\"{{FIELD}}\", "
"Self::{{OFFSET_NAME}}, {{IS_REQ}})?";
Expand Down
4 changes: 2 additions & 2 deletions tests/monster_test/my_game/example/monster_generated.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,7 @@ impl ::flatbuffers::Verifiable for Monster<'_> {
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<&'_ str>>>>("testarrayofstring", Self::VT_TESTARRAYOFSTRING, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<Monster>>>>("testarrayoftables", Self::VT_TESTARRAYOFTABLES, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<Monster>>("enemy", Self::VT_ENEMY, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::NestedFlatBuffer<Monster>>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<Stat>>("testempty", Self::VT_TESTEMPTY, false)?
.visit_field::<bool>("testbool", Self::VT_TESTBOOL, false)?
.visit_field::<i32>("testhashs32_fnv1", Self::VT_TESTHASHS32_FNV1, false)?
Expand Down Expand Up @@ -1119,7 +1119,7 @@ impl ::flatbuffers::Verifiable for Monster<'_> {
})?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, Color>>>("vector_of_enums", Self::VT_VECTOR_OF_ENUMS, false)?
.visit_field::<Race>("signed_enum", Self::VT_SIGNED_ENUM, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("testrequirednestedflatbuffer", Self::VT_TESTREQUIREDNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::NestedFlatBuffer<Monster>>>("testrequirednestedflatbuffer", Self::VT_TESTREQUIREDNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<Stat>>>>("scalar_key_sorted_tables", Self::VT_SCALAR_KEY_SORTED_TABLES, false)?
.visit_field::<Test>("native_inline", Self::VT_NATIVE_INLINE, false)?
.visit_field::<LongEnum>("long_enum_non_enum_default", Self::VT_LONG_ENUM_NON_ENUM_DEFAULT, false)?
Expand Down
4 changes: 2 additions & 2 deletions tests/monster_test_serialize/my_game/example/monster_generated.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ impl ::flatbuffers::Verifiable for Monster<'_> {
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<&'_ str>>>>("testarrayofstring", Self::VT_TESTARRAYOFSTRING, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<Monster>>>>("testarrayoftables", Self::VT_TESTARRAYOFTABLES, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<Monster>>("enemy", Self::VT_ENEMY, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::NestedFlatBuffer<Monster>>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<Stat>>("testempty", Self::VT_TESTEMPTY, false)?
.visit_field::<bool>("testbool", Self::VT_TESTBOOL, false)?
.visit_field::<i32>("testhashs32_fnv1", Self::VT_TESTHASHS32_FNV1, false)?
Expand Down Expand Up @@ -1121,7 +1121,7 @@ impl ::flatbuffers::Verifiable for Monster<'_> {
})?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, Color>>>("vector_of_enums", Self::VT_VECTOR_OF_ENUMS, false)?
.visit_field::<Race>("signed_enum", Self::VT_SIGNED_ENUM, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("testrequirednestedflatbuffer", Self::VT_TESTREQUIREDNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::NestedFlatBuffer<Monster>>>("testrequirednestedflatbuffer", Self::VT_TESTREQUIREDNESTEDFLATBUFFER, false)?
.visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset<Stat>>>>("scalar_key_sorted_tables", Self::VT_SCALAR_KEY_SORTED_TABLES, false)?
.visit_field::<Test>("native_inline", Self::VT_NATIVE_INLINE, false)?
.visit_field::<LongEnum>("long_enum_non_enum_default", Self::VT_LONG_ENUM_NON_ENUM_DEFAULT, false)?
Expand Down
Loading