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." + ); +}