Skip to content

Commit b31a3cd

Browse files
committed
fix: Add bounds checks in reflection API to prevent memory corruption
Add bounds checks to prevent out-of-bounds memory access when processing malformed binary schema (.bfbs) files through the reflection API. Changes: - src/reflection.cpp ForAllFields(): Add bounds check before using field->id() as array index. field->id() is a uint16_t (range 0-65535) but the array is sized to fields()->size(). A malformed schema with id >= fields()->size() would cause heap OOB write. - src/reflection.cpp VerifyObject(): Add minimum offset check for union fields before subtraction. field->offset() - sizeof(voffset_t) underflows when offset < 2, causing OOB read at offset 0xFFFF/0xFFFE from table pointer. - src/bfbs_gen.h FieldIdToIndex(): Same bounds check as ForAllFields() - both functions have identical vulnerable pattern. These checks add minimal overhead and protect against malformed schema files without breaking valid use cases. All existing tests pass.
1 parent 38df293 commit b31a3cd

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

src/bfbs_gen.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@ static std::vector<uint32_t> FieldIdToIndex(const reflection::Object* object) {
7272
// Create the mapping of field ID to the index into the vector.
7373
for (uint32_t i = 0; i < object->fields()->size(); ++i) {
7474
auto field = object->fields()->Get(i);
75-
field_index_by_id[field->id()] = i;
75+
// Bounds check: field->id() is a uint16_t from the schema and could
76+
// exceed fields()->size() in a malformed schema. Skip invalid IDs.
77+
if (field->id() < object->fields()->size()) {
78+
field_index_by_id[field->id()] = i;
79+
}
7680
}
7781

7882
return field_index_by_id;

src/reflection.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ static bool VerifyObject(flatbuffers::Verifier& v,
250250
}
251251
case reflection::Union: {
252252
// get union type from the prev field
253+
// Bounds check: offset must be >= sizeof(voffset_t) to avoid underflow
254+
if (field_def->offset() < sizeof(voffset_t)) {
255+
return false;
256+
}
253257
voffset_t utype_offset = field_def->offset() - sizeof(voffset_t);
254258
auto utype = table->GetField<uint8_t>(utype_offset, 0);
255259
auto uval = reinterpret_cast<const uint8_t*>(
@@ -384,7 +388,11 @@ void ForAllFields(const reflection::Object* object, bool reverse,
384388
// Create the mapping of field ID to the index into the vector.
385389
for (uint32_t i = 0; i < object->fields()->size(); ++i) {
386390
auto field = object->fields()->Get(i);
387-
field_to_id_map[field->id()] = i;
391+
// Bounds check: field->id() is a uint16_t from the schema and could
392+
// exceed fields()->size() in a malformed schema. Skip invalid IDs.
393+
if (field->id() < object->fields()->size()) {
394+
field_to_id_map[field->id()] = i;
395+
}
388396
}
389397

390398
for (size_t i = 0; i < field_to_id_map.size(); ++i) {

0 commit comments

Comments
 (0)