google / google/flatbuffers

Heap Out-of-Bounds Write via Unchecked `field->id()` Index in BFBS Schema Processing

Open
#8,950 1 comment 0 reactions 1 assignee Claimed by @jtdavis777 View on GitHub
bug c++ flatc stale
Dominant language
C++
Stars
26.5k
Forks
3.7k
PR merge metrics
No merged PRs in 30d

Description

## Summary

`FieldIdToIndex()` in `src/bfbs_gen.h` and `ForAllFields()` in `src/reflection.cpp` use `field->id()` as a direct array index into a vector sized by the number of fields, without bounds checking. Since `field->id()` is a `uint16_t` read from the binary FlatBuffer Schema (BFBS), a crafted `.bfbs` file can specify field IDs up to 65535 while having only 2 fields - causing a heap out-of-bounds write of up to 262,132 bytes past the allocation.

The FlatBuffers binary verifier (`reflection::VerifySchemaBuffer`) validates structural integrity but does **not** enforce the semantic constraint `field->id() < object->fields()->size()`. A malicious BFBS file passes verification, then triggers the OOB write when processed.

**This issue was reported to Google VRP and closed with permission to publicly disclose.**

## Affected Version

- **Repository:** [google/flatbuffers](https://github.com/google/flatbuffers)
- **Commit:** [`94d6b80`](https://github.com/google/flatbuffers/commit/94d6b8086b46bdff7da308aa2d3aebf336d29f55) (flatc 25.12.19)
- **Affected files:** `src/bfbs_gen.h`, `src/reflection.cpp`

## Root Cause

### Location 1: [`src/bfbs_gen.h` L68-79](https://github.com/google/flatbuffers/blob/94d6b8086b46bdff7da308aa2d3aebf336d29f55/src/bfbs_gen.h#L68-L79)

```cpp
static std::vector FieldIdToIndex(const reflection::Object* object) {
std::vector field_index_by_id;
field_index_by_id.resize(object->fields()->size()); // e.g., size = 2

for (uint32_t i = 0; i < object->fields()->size(); ++i) {
auto field = object->fields()->Get(i);
field_index_by_id[field->id()] = i; // BUG: field->id() can be 0..65535
}
return field_index_by_id;
}
```

### Location 2: [`src/reflection.cpp` L379-394](https://github.com/google/flatbuffers/blob/94d6b8086b46bdff7da308aa2d3aebf336d29f55/src/reflection.cpp#L379-L394)

Same pattern - `field_to_id_map[field->id()] = i` with no bounds check.

### Why Verification Doesn't Catch This

The generated verifier (`reflection_generated.h` L683-706) only checks:
```cpp
VerifyField(verifier, VT_ID, 2)
```
This validates the `id` field is a properly-located uint16_t. It does NOT check `id() < parent_object->fields()->size()`.

## Reproduction

### Build the harness

```bash
g++ -std=c++17 -fsanitize=address -g -I include/ -o oob_harness oob_harness.cpp
```

### Generate the patched BFBS

Use the included `patch_bfbs.py` to set `field->id()` to 65535 in a valid BFBS file.

### Run

```bash
ASAN_OPTIONS=detect_leaks=0 ./oob_harness oob_test_patched.bfbs
```

### ASAN Output

```
AddressSanitizer:DEADLYSIGNAL
==70269==ERROR: AddressSanitizer: BUS on unknown address
==70269==The signal is caused by a WRITE memory access.
#0 0x1021dd968 in FieldIdToIndex_Vulnerable(reflection::Object const*) oob_harness.cpp:47
#1 0x1021dcfe8 in main oob_harness.cpp:111

Register values:
x[1] = 0x000000000000ffff ← field->id() = 65535

SUMMARY: AddressSanitizer: BUS oob_harness.cpp:47 in FieldIdToIndex_Vulnerable
```

### PoC Output

```
[*] Object: OOBTest.TestTable (2 fields)
Field[0]: field_a, id=0, offset=4
Field[1]: field_b, id=65535, offset=6

[*] Calling FieldIdToIndex (equivalent to bfbs_gen.h:68-79)...
[!] OOB DETECTED: field->id()=65535 >= array_size=2
[!] About to write to index 65535 (OOB by 65533 elements = 262132 bytes)
```

## Impact

- **CWE-787** (Out-of-bounds Write)
- **CVSS 3.1:** 8.1 HIGH - `CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:H/A:H`
- Controlled write of a `uint32_t` value at a controlled offset (up to 262,140 bytes past allocation)
- Triggers via `flatc --lua`, `flatc --nim`, `flatc --annotate`, or any application using the reflection API's `ForAllFields()`
- CI/CD pipelines processing untrusted `.bfbs` files are directly exploitable

## Suggested Fix

Add bounds check on `field->id()`:

```cpp
for (uint32_t i = 0; i < num_fields; ++i) {
auto field = object->fields()->Get(i);
if (field->id() < num_fields) {
field_index_by_id[field->id()] = i;
}
}
```

Or add semantic validation to `VerifySchemaBuffer()` checking `field->id() < object->fields()->size()`.

## Related

- VRP-FB-014: `VerifySchemaBuffer()` semantic bypass enables this finding - the verifier passes on the malicious BFBS.

#

[asan_output.txt](https://github.com/user-attachments/files/25753007/asan_output.txt)
[oob_harness.cpp](https://github.com/user-attachments/files/25753006/oob_harness.cpp)
[patch_bfbs.py](https://github.com/user-attachments/files/25753005/patch_bfbs.py)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.