Fix heap OOB write in ForAllFields when field ID exceeds field count

[mohamedelabbas1996]

Description

Add bounds check in ForAllFields to validate that field->id() does not exceed the number of fields before using it as an index into field_to_id_map.

Without this check, a crafted .bfbs binary schema with a field ID larger than the field count causes an out-of-bounds write past the field_to_id_map vector at reflection.cpp:387. The crafted .bfbs passes reflection::VerifySchemaBuffer() — the verifier does not catch this because it only validates structural integrity, not the semantic constraint that field.id() < fields.size().

Root Cause

// reflection.cpp:381-388
std::vector<uint32_t> field_to_id_map;
field_to_id_map.resize(object->fields()->size());  // e.g., 3 slots (12 bytes)

for (uint32_t i = 0; i < object->fields()->size(); ++i) {
    auto field = object->fields()->Get(i);
    field_to_id_map[field->id()] = i;  // OOB WRITE if field->id() >= 3
}

field->id() is a uint16_t read from the .bfbs binary schema. With a crafted value of 100 and only 3 fields, the write goes 388 bytes past the 12-byte allocation into adjacent heap memory.

Proof of Concept

Step 1: Generate a normal .bfbs:

flatc --binary --schema test.fbs

Step 2: Patch one field's ID from 2 to 100 (a 2-byte change in the binary):

import struct
with open('poc.bfbs', 'rb') as f:
    data = bytearray(f.read())
# Find and patch the field ID at byte 172
struct.pack_into('<H', data, 172, 100)
with open('malicious.bfbs', 'wb') as f:
    f.write(data)

Step 3: Process with any tool that calls ForAllFields:

flatc --annotate malicious.bfbs

ASAN Output (macOS, confirmed)

==10444==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000260
WRITE of size 4 at 0x602000000260 thread T0
    #0 in flatbuffers::ForAllFields(reflection::Object const*, ...) reflection.cpp:387
    #1 in main poc_crash.cpp:100

0x602000000260 is located 388 bytes after 12-byte region [0x6020000000d0,0x6020000000dc)
SUMMARY: AddressSanitizer: heap-buffer-overflow reflection.cpp:387

Linux/glibc Verification (Ubuntu 22.04, confirmed)

On Linux without ASAN, the corruption is completely silent — no crash, no error:

Loaded malicious.bfbs (340 bytes)
Verifier: PASS
Fields: 3
  [0] id=1
  [1] id=100 OOB!
  [2] id=0
Calling ForAllFields...
  cb: name
  cb: hp
  cb: hp        ← 'mana' replaced by 'hp' due to heap corruption
Done

The program continues running with corrupted heap memory. The callback receives wrong field data (hp appears twice instead of mana), proving the corruption affects program behavior.

Verification Summary

Test	Platform	Result
Normal .bfbs	macOS	PASS, correct output
Normal .bfbs	Linux/glibc	PASS, correct output
Malicious .bfbs + ASAN	macOS	CRASH: heap-buffer-overflow WRITE
Malicious .bfbs no ASAN	Linux/glibc	Silent corruption, wrong output

Impact

Who is affected

ForAllFields is called from:

• binary_annotator.cpp — flatc --annotate
• bfbs_gen_lua.cpp — Lua code generation from .bfbs
• bfbs_gen_nim.cpp — Nim code generation from .bfbs
• Any application using the FlatBuffers reflection API

Attack scenario

In CI/CD pipelines that process .bfbs files from pull requests:

1. Attacker submits a PR with a crafted .bfbs file (340 bytes)
2. CI runs flatc --annotate or code generation on the .bfbs
3. Heap corruption occurs silently — build may succeed with wrong output
4. The corruption can cause wrong code generation, later crashes, or potential code execution via corrupted heap metadata

Severity justification

• Heap-buffer-overflow WRITE with attacker-controlled offset (CWE-122)
• Passes FlatBuffer verifier — defense layer is bypassed
• 340-byte trigger file — minimal attack payload
• Write offset range: 0 to 262,140 bytes past the allocation (controlled via field->id(), uint16 range 0-65535)
• Write value: controlled (field index, 0 to N-1 where N = number of fields)
• Silent without ASAN — no crash, no error, corruption goes undetected
• Same vulnerability class as OSV-2021-349 (FlatBuffers heap-buffer-overflow WRITE, rated HIGH)

Fix

Added if (field->id() < object->fields()->size()) guard before the array write. Fields with out-of-range IDs are silently skipped, preventing the OOB write while maintaining correct behavior for valid schemas.