From 438030c31a1cadc5f6f366a4c92cb8fd36f17593 Mon Sep 17 00:00:00 2001 From: sanil18 Date: Wed, 9 Sep 2026 02:36:51 +0545 Subject: [PATCH] [Rust] Verify nested_flatbuffer fields A field carrying the `nested_flatbuffer` attribute was verified only as `Vector`. The generated `..._nested_flatbuffer()` accessor follows those bytes as a FlatBuffer root without any further checking, so on a buffer that `root()` returned Ok for, every offset inside the nested buffer was still attacker-controlled. The consequence is that safe generated accessors read outside the buffer's allocation. Under Miri, `Monster::hp()` on such a buffer reports "attempting to access 2 bytes, but got alloc+0x9f which is only 1 byte from the end of the allocation". `read_scalar` guards that read with a debug_assert only, so release builds read past the end. A present-but- empty nested vector was likewise accepted and then panicked in the accessor. This contradicts docs/source/languages/rust.md, which states that the safe APIs are "intended to be safe for use on flatbuffers from untrusted sources", that "All of the safe Rust APIs ensure the verifier is run over these flatbuffers before accessing them", and that the generated accessors "access memory without any further bounds checking". C++ has verified nested buffers by default for years via Verifier::VerifyNestedFlatBuffer, so buffers Rust accepted here are already rejected by any C++ peer. Adds VerifierOptions::check_nested_flatbuffers (default true, matching C++), a NestedFlatBuffer verification marker, and emits it from the Rust generator for the verifier slot of such fields. Accessor return types are unchanged. Offsets inside a nested buffer are relative to its own start, so the verifier runs against the nested slice: `self.buffer` is swapped for it and restored afterwards. Swapping rather than building a second Verifier keeps depth, num_tables and apparent_size accumulating in one place, so a chain of nested buffers cannot reset them. That matters: porting the C++ design literally, with a fresh verifier and fresh budgets per level, would introduce a new DoS. Measured on a 200-level chain of nested Monsters, budgets reset gives "thread 'main' has overflowed its stack"; budgets carried gives DepthLimitReached on the same input. Nested bytes are also billed to apparent_size rather than verified for free, so nesting cannot multiply the verification work a small message buys. Tested: Rust suite and no_std suite pass (332 each); Miri clean on the nested paths; serde and both alloc checks unchanged; generated output verified byte-identical to patched-flatc output. Fixes the root cause reported in #8291. --- rust/flatbuffers/src/lib.rs | 4 +- rust/flatbuffers/src/verifier.rs | 67 +++- src/idl_gen_rust.cpp | 16 + .../my_game/example/monster_generated.rs | 4 +- .../my_game/example/monster_generated.rs | 4 +- .../rust_usage_test/tests/integration_test.rs | 310 ++++++++++++++++++ .../tests/nested_flatbuffer_invariant.rs | 214 ++++++++++++ 7 files changed, 612 insertions(+), 7 deletions(-) create mode 100644 tests/rust_usage_test/tests/nested_flatbuffer_invariant.rs diff --git a/rust/flatbuffers/src/lib.rs b/rust/flatbuffers/src/lib.rs index ed2bc173de..fa4dc67bec 100644 --- a/rust/flatbuffers/src/lib.rs +++ b/rust/flatbuffers/src/lib.rs @@ -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; diff --git a/rust/flatbuffers/src/verifier.rs b/rust/flatbuffers/src/verifier.rs index e992279d53..44f2e55dce 100644 --- a/rust/flatbuffers/src/verifier.rs +++ b/rust/flatbuffers/src/verifier.rs @@ -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; @@ -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 { @@ -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, } } } @@ -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(&mut self, range: Range) -> 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 = >::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( @@ -563,6 +602,32 @@ impl 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` +/// leaves that accessor reading unverified data. Verifying it as +/// `NestedFlatBuffer` 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`, +/// so this marker exists only to carry the extra verification and can never be +/// used to read data. +pub struct NestedFlatBuffer(PhantomData); + +impl Verifiable for NestedFlatBuffer { + #[inline] + fn run_verifier(v: &mut Verifier, pos: usize) -> Result<()> { + let range = verify_vector_range::(v, pos)?; + if !v.opts.check_nested_flatbuffers { + return Ok(()); + } + v.verify_nested_buffer::(range) + } +} + impl Verifiable for SkipSizePrefix { #[inline] fn run_verifier(v: &mut Verifier, pos: usize) -> Result<()> { diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index d62bbffa4d..0f7e4300e9 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -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}})?"; diff --git a/tests/monster_test/my_game/example/monster_generated.rs b/tests/monster_test/my_game/example/monster_generated.rs index d860e0613a..482af524f9 100644 --- a/tests/monster_test/my_game/example/monster_generated.rs +++ b/tests/monster_test/my_game/example/monster_generated.rs @@ -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>>>("testarrayoftables", Self::VT_TESTARRAYOFTABLES, false)? .visit_field::<::flatbuffers::ForwardsUOffset>("enemy", Self::VT_ENEMY, false)? - .visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)? + .visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::NestedFlatBuffer>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)? .visit_field::<::flatbuffers::ForwardsUOffset>("testempty", Self::VT_TESTEMPTY, false)? .visit_field::("testbool", Self::VT_TESTBOOL, false)? .visit_field::("testhashs32_fnv1", Self::VT_TESTHASHS32_FNV1, false)? @@ -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::("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>>("testrequirednestedflatbuffer", Self::VT_TESTREQUIREDNESTEDFLATBUFFER, false)? .visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset>>>("scalar_key_sorted_tables", Self::VT_SCALAR_KEY_SORTED_TABLES, false)? .visit_field::("native_inline", Self::VT_NATIVE_INLINE, false)? .visit_field::("long_enum_non_enum_default", Self::VT_LONG_ENUM_NON_ENUM_DEFAULT, false)? diff --git a/tests/monster_test_serialize/my_game/example/monster_generated.rs b/tests/monster_test_serialize/my_game/example/monster_generated.rs index 26652e1c95..b8a4fec098 100644 --- a/tests/monster_test_serialize/my_game/example/monster_generated.rs +++ b/tests/monster_test_serialize/my_game/example/monster_generated.rs @@ -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>>>("testarrayoftables", Self::VT_TESTARRAYOFTABLES, false)? .visit_field::<::flatbuffers::ForwardsUOffset>("enemy", Self::VT_ENEMY, false)? - .visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, u8>>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)? + .visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::NestedFlatBuffer>>("testnestedflatbuffer", Self::VT_TESTNESTEDFLATBUFFER, false)? .visit_field::<::flatbuffers::ForwardsUOffset>("testempty", Self::VT_TESTEMPTY, false)? .visit_field::("testbool", Self::VT_TESTBOOL, false)? .visit_field::("testhashs32_fnv1", Self::VT_TESTHASHS32_FNV1, false)? @@ -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::("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>>("testrequirednestedflatbuffer", Self::VT_TESTREQUIREDNESTEDFLATBUFFER, false)? .visit_field::<::flatbuffers::ForwardsUOffset<::flatbuffers::Vector<'_, ::flatbuffers::ForwardsUOffset>>>("scalar_key_sorted_tables", Self::VT_SCALAR_KEY_SORTED_TABLES, false)? .visit_field::("native_inline", Self::VT_NATIVE_INLINE, false)? .visit_field::("long_enum_non_enum_default", Self::VT_LONG_ENUM_NON_ENUM_DEFAULT, false)? diff --git a/tests/rust_usage_test/tests/integration_test.rs b/tests/rust_usage_test/tests/integration_test.rs index 21e5faf022..1285fc9f1a 100644 --- a/tests/rust_usage_test/tests/integration_test.rs +++ b/tests/rust_usage_test/tests/integration_test.rs @@ -509,6 +509,316 @@ fn verifier_in_too_deep() { assert!(flatbuffers::root_with_opts::(&opts, data).is_ok()); } +/// Builds a Monster whose `testnestedflatbuffer` field holds `payload`. +#[cfg(test)] +fn monster_with_nested_payload(payload: &[u8]) -> Vec { + use my_game::example::*; + let b = &mut flatbuffers::FlatBufferBuilder::new(); + let name = Some(b.create_string("outer")); + let nested = Some(b.create_vector::(payload)); + let m = Monster::create(b, &MonsterArgs { + name, // required field. + testnestedflatbuffer: nested, + ..Default::default() + }); + b.finish(m, None); + b.finished_data().to_vec() +} + +/// A well-formed nested Monster, so the nested check has something valid to accept. +#[cfg(test)] +fn valid_nested_monster(name: &str) -> Vec { + use my_game::example::*; + let b = &mut flatbuffers::FlatBufferBuilder::new(); + let n = Some(b.create_string(name)); + let m = Monster::create(b, &MonsterArgs { name: n, hp: 1234, ..Default::default() }); + b.finish(m, None); + b.finished_data().to_vec() +} + +#[test] +fn verifier_accepts_valid_nested_flatbuffer() { + use my_game::example::*; + let data = monster_with_nested_payload(&valid_nested_monster("nested")); + let m = flatbuffers::root::(&data).unwrap(); + let nested = m.testnestedflatbuffer_nested_flatbuffer().unwrap(); + assert_eq!(nested.name(), "nested"); + assert_eq!(nested.hp(), 1234); +} + +#[test] +fn verifier_rejects_nested_flatbuffer_with_invalid_utf8() { + use my_game::example::*; + // The nested buffer is structurally fine but its string is not UTF-8. + // Without verifying the nested buffer, the safe + // `testnestedflatbuffer_nested_flatbuffer()` accessor would hand out a + // `&str` that is not valid UTF-8. + let mut nested = valid_nested_monster("AAAA"); + let pos = nested.windows(4).position(|w| w == b"AAAA").unwrap(); + for i in 0..4 { nested[pos + i] = 0xFF; } + let data = monster_with_nested_payload(&nested); + assert!(flatbuffers::root::(&data).is_err()); +} + +#[test] +fn verifier_rejects_nested_flatbuffer_with_out_of_bounds_offsets() { + use my_game::example::*; + // uoffset 4 -> table at 4; soffset -7 -> vtable at 11, which leaves only one + // byte for a two-byte VOffsetT read. + let mut payload = Vec::new(); + payload.extend_from_slice(&4u32.to_le_bytes()); + payload.extend_from_slice(&(-7i32).to_le_bytes()); + payload.extend_from_slice(&[0xAA; 4]); + let data = monster_with_nested_payload(&payload); + assert!(flatbuffers::root::(&data).is_err()); +} + +#[test] +fn verifier_rejects_truncated_nested_flatbuffer() { + use my_game::example::*; + // Too short to even hold a root uoffset. + let data = monster_with_nested_payload(&[0u8; 2]); + assert!(flatbuffers::root::(&data).is_err()); +} + +#[test] +fn nested_flatbuffer_verification_can_be_disabled() { + use my_game::example::*; + let mut nested = valid_nested_monster("AAAA"); + let pos = nested.windows(4).position(|w| w == b"AAAA").unwrap(); + for i in 0..4 { nested[pos + i] = 0xFF; } + let data = monster_with_nested_payload(&nested); + + let mut opts = flatbuffers::VerifierOptions::default(); + assert!(opts.check_nested_flatbuffers); + assert!(flatbuffers::root_with_opts::(&opts, &data).is_err()); + + // Opting out restores the previous behaviour, matching the C++ option of + // the same name. Accessors are then unsound for untrusted input. + opts.check_nested_flatbuffers = false; + assert!(flatbuffers::root_with_opts::(&opts, &data).is_ok()); +} + +#[test] +#[cfg(not(miri))] // slow. +fn nested_flatbuffer_large_payload_still_verifies() { + use my_game::example::*; + // Verifying the nested buffer accounts for its contents against the outer + // budgets, so a legitimate, sizeable nested buffer must still verify under + // the default options. + let mut b = flatbuffers::FlatBufferBuilder::new(); + let n = b.create_string("big"); + let payload_bytes: Vec = (0..200_000u32).map(|i| i as u8).collect(); + let inv = b.create_vector::(&payload_bytes); + let m = Monster::create( + &mut b, + &MonsterArgs { name: Some(n), inventory: Some(inv), ..Default::default() }, + ); + b.finish(m, None); + let nested = b.finished_data().to_vec(); + assert!(nested.len() > 200_000); + + let data = monster_with_nested_payload(&nested); + let m = flatbuffers::root::(&data).expect("large nested buffer must still verify"); + let nm = m.testnestedflatbuffer_nested_flatbuffer().unwrap(); + assert_eq!(nm.name(), "big"); + assert_eq!(nm.inventory().unwrap().len(), 200_000); +} + +#[test] +#[cfg(not(miri))] // slow: builds 200 nested buffers. +fn nested_flatbuffer_chain_is_bounded() { + use my_game::example::*; + // Each level of nesting is verified with a verifier of its own. If the depth + // budget were not carried across that boundary, a chain of nested buffers + // would be unbounded and could exhaust the stack instead of being rejected. + let mut payload = valid_nested_monster("leaf"); + for _ in 0..200 { + payload = monster_with_nested_payload(&payload); + } + assert_eq!( + flatbuffers::root::(&payload).unwrap_err(), + flatbuffers::InvalidFlatbuffer::DepthLimitReached + ); +} + +#[test] +fn nested_flatbuffer_compat_empty_and_absent() { + use my_game::example::*; + // Absent field: unaffected, still verifies. + let b = &mut flatbuffers::FlatBufferBuilder::new(); + let name = Some(b.create_string("outer")); + let m = Monster::create(b, &MonsterArgs { name, ..Default::default() }); + b.finish(m, None); + assert!(flatbuffers::root::(b.finished_data()).is_ok()); + + // Present but empty: cannot hold a root, so it is now rejected. This + // matches C++, which requires a nested buffer to be at least + // FLATBUFFERS_MIN_BUFFER_SIZE. Previously it verified and then panicked in + // the accessor. + let data = monster_with_nested_payload(&[]); + assert!(flatbuffers::root::(&data).is_err()); +} + +#[test] +fn nested_flatbuffer_depth_counts_toward_max_depth() { + use my_game::example::*; + // The nested buffer is verified with its own Verifier, so its budgets must + // be carried over from the outer one; otherwise a chain of nested buffers + // could reset max_depth at every level. + let data = monster_with_nested_payload(&valid_nested_monster("nested")); + let mut opts = flatbuffers::VerifierOptions::default(); + opts.max_depth = 1; + assert_eq!( + flatbuffers::root_with_opts::(&opts, &data).unwrap_err(), + flatbuffers::InvalidFlatbuffer::DepthLimitReached + ); + opts.max_depth = 64; + assert!(flatbuffers::root_with_opts::(&opts, &data).is_ok()); +} + +/// A nested Monster carrying `n` bytes of inventory, so that verifying it costs +/// a measurable amount of the apparent-size budget. +#[cfg(test)] +fn nested_monster_with_inventory(n: usize) -> Vec { + use my_game::example::*; + let b = &mut flatbuffers::FlatBufferBuilder::new(); + let name = Some(b.create_string("inner")); + let inventory = Some(b.create_vector::(&vec![7u8; n])); + let m = Monster::create(b, &MonsterArgs { name, inventory, ..Default::default() }); + b.finish(m, None); + b.finished_data().to_vec() +} + +/// The smallest `max_apparent_size` that still accepts `data`. +#[cfg(test)] +fn min_apparent_size_budget(data: &[u8], check_nested: bool) -> usize { + use my_game::example::*; + let (mut lo, mut hi) = (0usize, 4 * 1024 * 1024usize); + while lo < hi { + let mid = lo + (hi - lo) / 2; + let mut opts = flatbuffers::VerifierOptions::default(); + opts.check_nested_flatbuffers = check_nested; + opts.max_apparent_size = mid; + if flatbuffers::root_with_opts::(&opts, data).is_ok() { + hi = mid; + } else { + lo = mid + 1; + } + } + lo +} + +/// Work done inside a nested buffer must be charged to the *outer* verifier's +/// apparent-size budget. If the nested buffer got a fresh budget, an attacker +/// could use nesting to multiply the amount of verification work a single +/// message can buy, which would make this fix a denial-of-service vector. +/// +/// The thresholds are measured rather than hard-coded so this cannot go stale +/// if the generated layout changes. +#[test] +fn nested_flatbuffer_work_counts_toward_apparent_size() { + use my_game::example::*; + let data = monster_with_nested_payload(&nested_monster_with_inventory(4096)); + + let without = min_apparent_size_budget(&data, false); + let with = min_apparent_size_budget(&data, true); + assert!( + with > without, + "verifying the nested buffer must consume apparent-size budget \ + (without={}, with={}); if these are equal the nested contents are \ + being verified for free and nesting is an amplifier", + without, + with + ); + + // At exactly the budget that suffices when the nested contents are skipped, + // verifying them must run out rather than proceed unbilled. + let mut opts = flatbuffers::VerifierOptions::default(); + opts.max_apparent_size = without; + assert_eq!( + flatbuffers::root_with_opts::(&opts, &data).unwrap_err(), + flatbuffers::InvalidFlatbuffer::ApparentSizeTooLarge + ); + // Control: the same budget is enough when the nested check is off, so the + // rejection above is caused by the nested work and not by a budget that was + // too small to begin with. + opts.check_nested_flatbuffers = false; + assert!(flatbuffers::root_with_opts::(&opts, &data).is_ok()); +} + +/// Tables inside a nested buffer must count toward `max_tables` for the same +/// reason: otherwise each nesting level would grant a fresh table budget. +#[test] +fn nested_flatbuffer_tables_count_toward_max_tables() { + use my_game::example::*; + let data = monster_with_nested_payload(&valid_nested_monster("nested")); + + // The outer Monster is one table; the nested one makes two. + let mut opts = flatbuffers::VerifierOptions::default(); + opts.max_tables = 1; + assert_eq!( + flatbuffers::root_with_opts::(&opts, &data).unwrap_err(), + flatbuffers::InvalidFlatbuffer::TooManyTables + ); + // Control: one table is enough once the nested contents are not verified, + // so the limit above is reached by the nested table and not by the outer one. + opts.check_nested_flatbuffers = false; + assert!(flatbuffers::root_with_opts::(&opts, &data).is_ok()); + + opts.check_nested_flatbuffers = true; + opts.max_tables = 2; + assert!(flatbuffers::root_with_opts::(&opts, &data).is_ok()); +} + +#[test] +fn nested_flatbuffer_is_verified_recursively() { + use my_game::example::*; + // A nested buffer can itself hold a nested buffer, so verification has to + // recurse all the way down rather than stopping at the first level. Build + // outer -> nested Monster -> its own nested buffer holding malformed bytes, + // and confirm the outer verifier rejects it. + let malformed = [0xFFu8, 0xFF, 0xFF, 0xFF, 0x01, 0, 0, 0, 0xAA, 0xBB]; + let mut b = flatbuffers::FlatBufferBuilder::new(); + let name = Some(b.create_string("inner")); + let inner = Some(b.create_vector::(&malformed)); + let m = Monster::create(&mut b, &MonsterArgs { + name, + testnestedflatbuffer: inner, + ..Default::default() + }); + b.finish(m, None); + let level_one = b.finished_data().to_vec(); + + let data = monster_with_nested_payload(&level_one); + assert!( + flatbuffers::root::(&data).is_err(), + "the malformed innermost buffer must be caught by recursive verification" + ); +} + +#[test] +fn nested_flatbuffer_verified_on_size_prefixed_root() { + use my_game::example::*; + // The nested check lives in the `Verifiable` impl, so it must apply on every + // safe entry point, not just `root`. Confirm `size_prefixed_root` rejects a + // malformed nested buffer too. + let malformed = [0xFFu8, 0xFF, 0xFF, 0xFF, 0x01, 0, 0, 0, 0xAA, 0xBB]; + let mut b = flatbuffers::FlatBufferBuilder::new(); + let name = Some(b.create_string("outer")); + let nested = Some(b.create_vector::(&malformed)); + let m = Monster::create(&mut b, &MonsterArgs { + name, + testnestedflatbuffer: nested, + ..Default::default() + }); + b.finish_size_prefixed(m, None); + assert!( + flatbuffers::size_prefixed_root::(b.finished_data()).is_err(), + "size_prefixed_root must verify nested buffers like root does" + ); +} + #[cfg(test)] mod generated_constants { extern crate flatbuffers; diff --git a/tests/rust_usage_test/tests/nested_flatbuffer_invariant.rs b/tests/rust_usage_test/tests/nested_flatbuffer_invariant.rs new file mode 100644 index 0000000000..19935f274e --- /dev/null +++ b/tests/rust_usage_test/tests/nested_flatbuffer_invariant.rs @@ -0,0 +1,214 @@ +//! Differential check for the invariant the Rust docs promise: +//! +//! "All of the safe Rust APIs ensure the verifier is run over these +//! flatbuffers before accessing them." +//! +//! Concretely: if `root::()` accepts a buffer, then reading the nested +//! flatbuffer out of it through the generated accessor must not panic and must +//! not produce a `&str` that is not valid UTF-8. +//! +//! The nested payload is entirely attacker-controlled, so this mutates it and +//! checks the invariant over every mutant the verifier accepts. Mutation is +//! driven by a fixed-seed LCG so failures are reproducible and CI is stable. +#![allow(dead_code, unused_imports)] + +#[allow(dead_code, unused_imports, clippy::approx_constant)] +#[path = "../../monster_test/mod.rs"] +mod monster_test_generated; +pub use monster_test_generated::my_game; +use my_game::example::{Monster, MonsterArgs}; + +use std::panic::{catch_unwind, AssertUnwindSafe}; + +fn valid_nested(name: &str) -> Vec { + let mut b = flatbuffers::FlatBufferBuilder::new(); + let n = b.create_string(name); + let inv = b.create_vector::(&[1, 2, 3, 4]); + let m = Monster::create( + &mut b, + &MonsterArgs { name: Some(n), hp: 1234, inventory: Some(inv), ..Default::default() }, + ); + b.finish(m, None); + b.finished_data().to_vec() +} + +fn outer_with(payload: &[u8]) -> Vec { + let mut b = flatbuffers::FlatBufferBuilder::new(); + let name = b.create_string("outer"); + let nested = b.create_vector::(payload); + let m = Monster::create( + &mut b, + &MonsterArgs { name: Some(name), testnestedflatbuffer: Some(nested), ..Default::default() }, + ); + b.finish(m, None); + b.finished_data().to_vec() +} + +/// Exercises the nested buffer the way an application would. Returns Err if the +/// product misbehaved: a panic, or a `&str` that is not valid UTF-8. +fn traverse(data: &[u8], opts: &flatbuffers::VerifierOptions) -> Result { + let monster = match flatbuffers::root_with_opts::(opts, data) { + Ok(m) => m, + Err(_) => return Ok(false), // rejected: nothing to check + }; + let res = catch_unwind(AssertUnwindSafe(|| { + let nested = match monster.testnestedflatbuffer_nested_flatbuffer() { + Some(n) => n, + None => return Ok(()), + }; + // Ordinary field reads, the same as any application would do. + let name: &str = nested.name(); + if core::str::from_utf8(name.as_bytes()).is_err() { + return Err(format!("accessor produced non-UTF-8 &str: {:02X?}", name.as_bytes())); + } + let _ = nested.hp(); + let _ = nested.mana(); + if let Some(inv) = nested.inventory() { + for i in 0..inv.len() { + let _ = inv.get(i); + } + } + if let Some(v) = nested.testarrayofstring() { + for i in 0..v.len() { + let s = v.get(i); + if core::str::from_utf8(s.as_bytes()).is_err() { + return Err("vector element produced non-UTF-8 &str".to_string()); + } + } + } + Ok(()) + })); + match res { + Err(_) => Err("panicked while reading a verified buffer".to_string()), + Ok(Err(e)) => Err(e), + Ok(Ok(())) => Ok(true), + } +} + +/// Result of one mutation sweep. +struct Sweep { + /// Buffers the verifier accepted and which traversed cleanly. + accepted: usize, + /// Buffers the verifier accepted but which then misbehaved. This is the + /// number that must be zero. + misbehaved: usize, + /// First few distinct failures, for the assertion message. Kept separate + /// from `misbehaved` so the count is never truncated by the sample limit. + examples: Vec, +} + +/// Deterministic mutation sweep over the attacker-controlled nested payload. +fn sweep(opts: &flatbuffers::VerifierOptions) -> Sweep { + let base = valid_nested("AAAA"); + let mut state: u64 = 0x5EED_1234_ABCD_0001; + let mut next = || { + state = state.wrapping_mul(6364136223846793005).wrapping_add(1442695040888963407); + (state >> 33) as usize + }; + let mut out = Sweep { accepted: 0, misbehaved: 0, examples: Vec::new() }; + for _ in 0..4000 { + let mut payload = base.clone(); + // Corrupt one to three bytes anywhere in the nested buffer. + let n = 1 + next() % 3; + for _ in 0..n { + let idx = next() % payload.len(); + payload[idx] = (next() % 256) as u8; + } + let data = outer_with(&payload); + match traverse(&data, opts) { + Ok(true) => out.accepted += 1, + Ok(false) => {} + Err(e) => { + out.misbehaved += 1; + if out.examples.len() < 5 && !out.examples.contains(&e) { + out.examples.push(e); + } + } + } + } + out +} + +/// With nested verification on (the default), every buffer the verifier accepts +/// must survive a full traversal. +#[test] +#[cfg(not(miri))] // slow. +fn accepted_buffers_are_safe_to_traverse() { + let opts = flatbuffers::VerifierOptions::default(); + assert!(opts.check_nested_flatbuffers, "nested checking must default to on"); + let s = sweep(&opts); + assert!(s.accepted > 0, "no buffer was accepted; the sweep would prove nothing"); + assert_eq!( + s.misbehaved, 0, + "verifier accepted {} buffers, {} of which misbehaved when read: {:?}", + s.accepted, s.misbehaved, s.examples + ); +} + +/// The verifier must never panic, whatever it is handed. The sweep above only +/// corrupts the nested payload, leaving the outer buffer well-formed, so it +/// never exercises a hostile *vector header* -- and the length field of that +/// header is what determines the range handed to the nested verifier. +/// +/// This corrupts the whole outer buffer instead, so the length, the offsets and +/// the vtable are all attacker-controlled. Any panic here is a denial-of-service +/// bug, so the assertion is simply that verification always returns. +#[test] +#[cfg(not(miri))] // slow. +fn verifier_never_panics_on_a_corrupt_outer_buffer() { + let base = outer_with(&valid_nested("AAAA")); + let mut state: u64 = 0xA11C_E501_2345_6789; + let mut next = || { + state = state.wrapping_mul(6364136223846793005).wrapping_add(1442695040888963407); + (state >> 33) as usize + }; + + let opts = flatbuffers::VerifierOptions::default(); + let mut accepted = 0usize; + let mut rejected = 0usize; + for _ in 0..20000 { + let mut data = base.clone(); + let n = 1 + next() % 4; + for _ in 0..n { + let idx = next() % data.len(); + data[idx] = (next() % 256) as u8; + } + let res = catch_unwind(AssertUnwindSafe(|| { + flatbuffers::root_with_opts::(&opts, &data).is_ok() + })); + match res { + Ok(true) => accepted += 1, + Ok(false) => rejected += 1, + Err(_) => panic!( + "verifier panicked on a corrupt buffer; this is a DoS. Input: {:02X?}", + data + ), + } + } + // Non-vacuity: the sweep has to actually reach both outcomes, otherwise it + // is not exercising the verifier's decision paths at all. + assert!( + accepted > 0 && rejected > 0, + "sweep reached only one outcome (accepted={}, rejected={}), so it is not \ + exercising the verifier's decision paths", + accepted, + rejected + ); +} + +/// Control: the same sweep with nested verification disabled reproduces the +/// old behaviour, which does misbehave. This keeps the test above honest -- if +/// it ever passes for the wrong reason, this one will stop failing to fail. +#[test] +#[cfg(not(miri))] // slow. +fn control_disabling_nested_check_reintroduces_the_problem() { + let mut opts = flatbuffers::VerifierOptions::default(); + opts.check_nested_flatbuffers = false; + let s = sweep(&opts); + assert!( + s.misbehaved > 0, + "expected the unverified path to misbehave on at least one mutant. If it \ + no longer does, the sweep has stopped reaching the behaviour it is meant \ + to guard and the test above proves nothing." + ); +}