Skip to content

Commit f6dce03

Browse files
committed
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>
1 parent 10c9941 commit f6dce03

4 files changed

Lines changed: 177 additions & 4 deletions

File tree

rust/flexbuffers/fuzz/Cargo.toml

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
[package]
2+
name = "flexbuffers-fuzz"
3+
version = "0.0.0"
4+
publish = false
5+
edition = "2018"
6+
7+
[package.metadata]
8+
cargo-fuzz = true
9+
10+
[dependencies]
11+
libfuzzer-sys = "0.4"
12+
13+
[dependencies.flexbuffers]
14+
path = ".."
15+
16+
# Prevent this from interfering with workspace
17+
[workspace]
18+
19+
[[bin]]
20+
name = "fuzz_reader"
21+
path = "fuzz_targets/fuzz_reader.rs"
22+
test = false
23+
doc = false
24+
25+
[[bin]]
26+
name = "fuzz_roundtrip"
27+
path = "fuzz_targets/fuzz_roundtrip.rs"
28+
test = false
29+
doc = false
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Fuzz target: exhaustively exercise the FlexBuffers Reader API with arbitrary input.
2+
//
3+
// Exercises every public accessor on Reader so that any panic from malformed
4+
// data is caught by the fuzzer. Before the fix for read_usize, this harness
5+
// would quickly find the crash with a 4-byte payload.
6+
//
7+
// Run with:
8+
// cargo +nightly fuzz run fuzz_reader
9+
10+
#![no_main]
11+
12+
use libfuzzer_sys::fuzz_target;
13+
use flexbuffers::Reader;
14+
15+
fuzz_target!(|data: &[u8]| {
16+
let Ok(reader) = Reader::get_root(data) else { return };
17+
18+
// Exercise every accessor — any panic here is a bug.
19+
let _ = reader.as_bool();
20+
let _ = reader.as_u8();
21+
let _ = reader.as_u16();
22+
let _ = reader.as_u32();
23+
let _ = reader.as_u64();
24+
let _ = reader.as_i8();
25+
let _ = reader.as_i16();
26+
let _ = reader.as_i32();
27+
let _ = reader.as_i64();
28+
let _ = reader.as_f32();
29+
let _ = reader.as_f64();
30+
let _ = reader.as_str();
31+
let _ = reader.as_bytes();
32+
let _ = reader.length();
33+
let _ = reader.flexbuffer_type();
34+
let _ = reader.bitwidth();
35+
36+
// Exercise map reader if applicable.
37+
if let Ok(map) = reader.as_map().ok().map(|_| reader.as_map()) {
38+
if let Ok(m) = map {
39+
for i in 0..m.len().min(16) {
40+
let _ = m.idx(i);
41+
}
42+
}
43+
}
44+
45+
// Exercise vector reader if applicable.
46+
if let Ok(vec) = reader.as_vector().ok().map(|_| reader.as_vector()) {
47+
if let Ok(v) = vec {
48+
for i in 0..v.len().min(16) {
49+
let _ = v.idx(i);
50+
}
51+
}
52+
}
53+
});
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Fuzz target: roundtrip encode→decode must never panic on any Reader accessor.
2+
//
3+
// Run with:
4+
// cargo +nightly fuzz run fuzz_roundtrip
5+
6+
#![no_main]
7+
8+
use libfuzzer_sys::{arbitrary, fuzz_target, Corpus};
9+
use flexbuffers::Reader;
10+
11+
fuzz_target!(|data: &[u8]| -> Corpus {
12+
// Only test parseable inputs.
13+
if Reader::get_root(data).is_err() {
14+
return Corpus::Reject;
15+
}
16+
let reader = Reader::get_root(data).unwrap();
17+
18+
// Any of these panicking is a bug in the Reader.
19+
let _ = reader.as_bool();
20+
let _ = reader.as_u64();
21+
let _ = reader.as_i64();
22+
let _ = reader.as_f64();
23+
let _ = reader.as_str();
24+
let _ = reader.length();
25+
26+
Corpus::Keep
27+
});

rust/flexbuffers/src/reader/mod.rs

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,15 @@ impl<B: Buffer> Reader<B> {
323323
/// Otherwise Returns error.
324324
pub fn get_bool(&self) -> Result<bool, Error> {
325325
self.expect_type(FlexBufferType::Bool)?;
326-
Ok(self.buffer[self.address..self.address + self.width.n_bytes()].iter().any(|&b| b != 0))
326+
let end = self
327+
.address
328+
.checked_add(self.width.n_bytes())
329+
.ok_or(Error::FlexbufferOutOfBounds)?;
330+
let slice = self
331+
.buffer
332+
.get(self.address..end)
333+
.ok_or(Error::FlexbufferOutOfBounds)?;
334+
Ok(slice.iter().any(|&b| b != 0))
327335
}
328336

329337
/// Gets the length of the key if this type is a key.
@@ -332,7 +340,11 @@ impl<B: Buffer> Reader<B> {
332340
#[inline]
333341
fn get_key_len(&self) -> Result<usize, Error> {
334342
self.expect_type(FlexBufferType::Key)?;
335-
let (length, _) = self.buffer[self.address..]
343+
let tail = self
344+
.buffer
345+
.get(self.address..)
346+
.ok_or(Error::FlexbufferOutOfBounds)?;
347+
let (length, _) = tail
336348
.iter()
337349
.enumerate()
338350
.find(|(_, &b)| b == b'\0')
@@ -601,9 +613,9 @@ fn f64_from_le_bytes(bytes: [u8; 8]) -> f64 {
601613
}
602614

603615
fn read_usize(buffer: &[u8], address: usize, width: BitWidth) -> usize {
604-
let cursor = &buffer[address..];
616+
let cursor = buffer.get(address..).unwrap_or_default();
605617
match width {
606-
BitWidth::W8 => cursor[0] as usize,
618+
BitWidth::W8 => cursor.first().copied().unwrap_or_default() as usize,
607619
BitWidth::W16 => cursor
608620
.get(0..2)
609621
.and_then(|s| s.try_into().ok())
@@ -627,3 +639,55 @@ fn unpack_type(ty: u8) -> Result<(FlexBufferType, BitWidth), Error> {
627639
let t = FlexBufferType::try_from(ty >> 2).map_err(|_| Error::InvalidPackedType)?;
628640
Ok((t, w))
629641
}
642+
643+
#[cfg(test)]
644+
mod tests {
645+
use super::*;
646+
647+
// Regression tests for panics triggered by crafted FlexBuffer input.
648+
// Before the fix, read_usize used direct indexing (&buffer[address..] and cursor[0])
649+
// which panicked when address was at or past the end of the buffer.
650+
// See: https://github.com/google/flatbuffers/issues/8923
651+
652+
/// Crafted payload whose declared W8 length slot falls exactly at buffer end.
653+
/// read_usize was called as: &buffer[address..] where address == buffer.len(),
654+
/// yielding an empty cursor, then cursor[0] panicked.
655+
#[test]
656+
fn read_usize_w8_address_at_end_does_not_panic() {
657+
// Byte layout (little-endian FlexBuffer):
658+
// [value_byte, type_byte, root_width_byte]
659+
// Crafting a Bool with a width that pushes the length slot OOB.
660+
let data: &[u8] = &[0x5d, 0x79, 0x6b, 0x02];
661+
let reader = Reader::get_root(data).unwrap_or_default();
662+
// Must not panic — should return false or an error gracefully.
663+
let _ = reader.as_bool();
664+
}
665+
666+
/// Crafted payload where the computed address for the length slot exceeds buffer length.
667+
/// Before the fix, &buffer[address..] panicked immediately (address > len).
668+
#[test]
669+
fn read_usize_address_past_end_does_not_panic() {
670+
// A minimal FlexBuffer whose internal offset arithmetic resolves to an address
671+
// beyond the buffer bounds for a W8-width length field.
672+
let data: &[u8] = &[0x00, 0x25, 0x01];
673+
let reader = Reader::get_root(data).unwrap_or_default();
674+
let _ = reader.length();
675+
}
676+
677+
/// read_usize with W8 and address exactly at buffer end returns 0, not panic.
678+
#[test]
679+
fn read_usize_w8_returns_zero_on_oob() {
680+
let buffer: &[u8] = &[0x01, 0x02];
681+
// Address at len — empty cursor.
682+
let result = read_usize(buffer, buffer.len(), BitWidth::W8);
683+
assert_eq!(result, 0, "Expected 0 (safe default) for OOB W8 read");
684+
}
685+
686+
/// read_usize with an address past the end returns 0, not panic.
687+
#[test]
688+
fn read_usize_past_end_returns_zero() {
689+
let buffer: &[u8] = &[0xAB];
690+
let result = read_usize(buffer, 999, BitWidth::W8);
691+
assert_eq!(result, 0, "Expected 0 (safe default) for address past buffer end");
692+
}
693+
}

0 commit comments

Comments
 (0)