From f6dce0328f073b1a89389910957b29a0e252ac77 Mon Sep 17 00:00:00 2001 From: canuk40 <258483010+canuk40@users.noreply.github.com> Date: Sun, 15 Mar 2026 11:17:13 -0400 Subject: [PATCH] fix(flexbuffers): prevent Reader panics from crafted input Three direct-index slice panics in rust/flexbuffers/src/reader/mod.rs are reachable from untrusted FlexBuffer input: 1. get_bool(): buffer[address..address+n_bytes] panics when OOB 2. get_key_len(): buffer[address..] panics when address > len 3. read_usize(): &buffer[address..] and cursor[0] panic when OOB (W16/W32/W64 arms already used .get() safely; W8 did not) Fix: replace all three with .get()-based indexing that returns Err(FlexbufferOutOfBounds) or 0 instead of panicking. Repro (from issue #8923): let data = &[0x5d, 0x79, 0x6b, 0x02]; let r = flexbuffers::Reader::get_root(data).unwrap(); let _ = r.as_bool(); // panicked before this fix Also adds: - 4 regression tests in reader::tests - cargo-fuzz harness with fuzz_reader and fuzz_roundtrip targets Closes #8923. Extends #8924 to cover read_usize() and adds fuzzing. Signed-off-by: canuk40 <258483010+canuk40@users.noreply.github.com> --- rust/flexbuffers/fuzz/Cargo.toml | 29 ++++++++ .../fuzz/fuzz_targets/fuzz_reader.rs | 53 ++++++++++++++ .../fuzz/fuzz_targets/fuzz_roundtrip.rs | 27 +++++++ rust/flexbuffers/src/reader/mod.rs | 72 +++++++++++++++++-- 4 files changed, 177 insertions(+), 4 deletions(-) create mode 100644 rust/flexbuffers/fuzz/Cargo.toml create mode 100644 rust/flexbuffers/fuzz/fuzz_targets/fuzz_reader.rs create mode 100644 rust/flexbuffers/fuzz/fuzz_targets/fuzz_roundtrip.rs diff --git a/rust/flexbuffers/fuzz/Cargo.toml b/rust/flexbuffers/fuzz/Cargo.toml new file mode 100644 index 00000000000..dc124fae5f7 --- /dev/null +++ b/rust/flexbuffers/fuzz/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "flexbuffers-fuzz" +version = "0.0.0" +publish = false +edition = "2018" + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4" + +[dependencies.flexbuffers] +path = ".." + +# Prevent this from interfering with workspace +[workspace] + +[[bin]] +name = "fuzz_reader" +path = "fuzz_targets/fuzz_reader.rs" +test = false +doc = false + +[[bin]] +name = "fuzz_roundtrip" +path = "fuzz_targets/fuzz_roundtrip.rs" +test = false +doc = false diff --git a/rust/flexbuffers/fuzz/fuzz_targets/fuzz_reader.rs b/rust/flexbuffers/fuzz/fuzz_targets/fuzz_reader.rs new file mode 100644 index 00000000000..f8fb35c6a79 --- /dev/null +++ b/rust/flexbuffers/fuzz/fuzz_targets/fuzz_reader.rs @@ -0,0 +1,53 @@ +// Fuzz target: exhaustively exercise the FlexBuffers Reader API with arbitrary input. +// +// Exercises every public accessor on Reader so that any panic from malformed +// data is caught by the fuzzer. Before the fix for read_usize, this harness +// would quickly find the crash with a 4-byte payload. +// +// Run with: +// cargo +nightly fuzz run fuzz_reader + +#![no_main] + +use libfuzzer_sys::fuzz_target; +use flexbuffers::Reader; + +fuzz_target!(|data: &[u8]| { + let Ok(reader) = Reader::get_root(data) else { return }; + + // Exercise every accessor — any panic here is a bug. + let _ = reader.as_bool(); + let _ = reader.as_u8(); + let _ = reader.as_u16(); + let _ = reader.as_u32(); + let _ = reader.as_u64(); + let _ = reader.as_i8(); + let _ = reader.as_i16(); + let _ = reader.as_i32(); + let _ = reader.as_i64(); + let _ = reader.as_f32(); + let _ = reader.as_f64(); + let _ = reader.as_str(); + let _ = reader.as_bytes(); + let _ = reader.length(); + let _ = reader.flexbuffer_type(); + let _ = reader.bitwidth(); + + // Exercise map reader if applicable. + if let Ok(map) = reader.as_map().ok().map(|_| reader.as_map()) { + if let Ok(m) = map { + for i in 0..m.len().min(16) { + let _ = m.idx(i); + } + } + } + + // Exercise vector reader if applicable. + if let Ok(vec) = reader.as_vector().ok().map(|_| reader.as_vector()) { + if let Ok(v) = vec { + for i in 0..v.len().min(16) { + let _ = v.idx(i); + } + } + } +}); diff --git a/rust/flexbuffers/fuzz/fuzz_targets/fuzz_roundtrip.rs b/rust/flexbuffers/fuzz/fuzz_targets/fuzz_roundtrip.rs new file mode 100644 index 00000000000..1f3e3cbcc8a --- /dev/null +++ b/rust/flexbuffers/fuzz/fuzz_targets/fuzz_roundtrip.rs @@ -0,0 +1,27 @@ +// Fuzz target: roundtrip encode→decode must never panic on any Reader accessor. +// +// Run with: +// cargo +nightly fuzz run fuzz_roundtrip + +#![no_main] + +use libfuzzer_sys::{arbitrary, fuzz_target, Corpus}; +use flexbuffers::Reader; + +fuzz_target!(|data: &[u8]| -> Corpus { + // Only test parseable inputs. + if Reader::get_root(data).is_err() { + return Corpus::Reject; + } + let reader = Reader::get_root(data).unwrap(); + + // Any of these panicking is a bug in the Reader. + let _ = reader.as_bool(); + let _ = reader.as_u64(); + let _ = reader.as_i64(); + let _ = reader.as_f64(); + let _ = reader.as_str(); + let _ = reader.length(); + + Corpus::Keep +}); diff --git a/rust/flexbuffers/src/reader/mod.rs b/rust/flexbuffers/src/reader/mod.rs index 5035dea6c6f..5617ee36ccf 100644 --- a/rust/flexbuffers/src/reader/mod.rs +++ b/rust/flexbuffers/src/reader/mod.rs @@ -323,7 +323,15 @@ impl Reader { /// Otherwise Returns error. pub fn get_bool(&self) -> Result { self.expect_type(FlexBufferType::Bool)?; - Ok(self.buffer[self.address..self.address + self.width.n_bytes()].iter().any(|&b| b != 0)) + let end = self + .address + .checked_add(self.width.n_bytes()) + .ok_or(Error::FlexbufferOutOfBounds)?; + let slice = self + .buffer + .get(self.address..end) + .ok_or(Error::FlexbufferOutOfBounds)?; + Ok(slice.iter().any(|&b| b != 0)) } /// Gets the length of the key if this type is a key. @@ -332,7 +340,11 @@ impl Reader { #[inline] fn get_key_len(&self) -> Result { self.expect_type(FlexBufferType::Key)?; - let (length, _) = self.buffer[self.address..] + let tail = self + .buffer + .get(self.address..) + .ok_or(Error::FlexbufferOutOfBounds)?; + let (length, _) = tail .iter() .enumerate() .find(|(_, &b)| b == b'\0') @@ -601,9 +613,9 @@ fn f64_from_le_bytes(bytes: [u8; 8]) -> f64 { } fn read_usize(buffer: &[u8], address: usize, width: BitWidth) -> usize { - let cursor = &buffer[address..]; + let cursor = buffer.get(address..).unwrap_or_default(); match width { - BitWidth::W8 => cursor[0] as usize, + BitWidth::W8 => cursor.first().copied().unwrap_or_default() as usize, BitWidth::W16 => cursor .get(0..2) .and_then(|s| s.try_into().ok()) @@ -627,3 +639,55 @@ fn unpack_type(ty: u8) -> Result<(FlexBufferType, BitWidth), Error> { let t = FlexBufferType::try_from(ty >> 2).map_err(|_| Error::InvalidPackedType)?; Ok((t, w)) } + +#[cfg(test)] +mod tests { + use super::*; + + // Regression tests for panics triggered by crafted FlexBuffer input. + // Before the fix, read_usize used direct indexing (&buffer[address..] and cursor[0]) + // which panicked when address was at or past the end of the buffer. + // See: https://github.com/google/flatbuffers/issues/8923 + + /// Crafted payload whose declared W8 length slot falls exactly at buffer end. + /// read_usize was called as: &buffer[address..] where address == buffer.len(), + /// yielding an empty cursor, then cursor[0] panicked. + #[test] + fn read_usize_w8_address_at_end_does_not_panic() { + // Byte layout (little-endian FlexBuffer): + // [value_byte, type_byte, root_width_byte] + // Crafting a Bool with a width that pushes the length slot OOB. + let data: &[u8] = &[0x5d, 0x79, 0x6b, 0x02]; + let reader = Reader::get_root(data).unwrap_or_default(); + // Must not panic — should return false or an error gracefully. + let _ = reader.as_bool(); + } + + /// Crafted payload where the computed address for the length slot exceeds buffer length. + /// Before the fix, &buffer[address..] panicked immediately (address > len). + #[test] + fn read_usize_address_past_end_does_not_panic() { + // A minimal FlexBuffer whose internal offset arithmetic resolves to an address + // beyond the buffer bounds for a W8-width length field. + let data: &[u8] = &[0x00, 0x25, 0x01]; + let reader = Reader::get_root(data).unwrap_or_default(); + let _ = reader.length(); + } + + /// read_usize with W8 and address exactly at buffer end returns 0, not panic. + #[test] + fn read_usize_w8_returns_zero_on_oob() { + let buffer: &[u8] = &[0x01, 0x02]; + // Address at len — empty cursor. + let result = read_usize(buffer, buffer.len(), BitWidth::W8); + assert_eq!(result, 0, "Expected 0 (safe default) for OOB W8 read"); + } + + /// read_usize with an address past the end returns 0, not panic. + #[test] + fn read_usize_past_end_returns_zero() { + let buffer: &[u8] = &[0xAB]; + let result = read_usize(buffer, 999, BitWidth::W8); + assert_eq!(result, 0, "Expected 0 (safe default) for address past buffer end"); + } +}