From 393aa03dff02230177a258672159141d877d9376 Mon Sep 17 00:00:00 2001 From: unpredictable21 Date: Tue, 1 Sep 2026 13:35:44 +0800 Subject: [PATCH] Bound out-of-range Object/Enum index lookups in BinaryAnnotator When flatc --annotate consumes a .bfbs schema, BinaryAnnotator walks each Field's Type.index and dereferences schema_->objects()->Get(field->type()->index()) (and the matching enums vector) without first validating that the index is in range. The reflection::VerifySchemaBuffer that runs before annotation only checks structural integrity of the schema (offsets, sizes, alignment, vector bounds); it never validates that a Field.type.index references a slot in schema->objects() or schema->enums(). A schema with Type { base_type = Obj, index = N } where N >= schema->objects()->size() (and similarly for unions and enum-driven indexes) reaches one of eight unguarded sinks in binary_annotator.cpp / binary_annotator.h. Each sink reads Vector::Get(N) where N is far beyond the underlying heap allocation. In a release build without FLATBUFFERS_ASSERT, the read proceeds past the heap buffer and into adjacent heap pages; depending on the index value this either crosses into unmapped memory (SEGV) or reads attacker-influenced heap contents. This change adds two helpers, BinaryAnnotator::GetObject and BinaryAnnotator::GetEnum, which return nullptr when the index is negative or beyond the schema's objects() / enums() size. Every unguarded sink is rewired through these helpers and bails out cleanly when the index is out of bounds. The BuildStruct / BuildVector / BuildUnion paths stop traversing when the bound is exceeded; the BuildTable Obj path emits a generic (unknown) annotation for the offending field so downstream regions remain well-formed. --- src/binary_annotator.cpp | 47 ++++++++++++++++++++++++++++++++-------- src/binary_annotator.h | 30 +++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/binary_annotator.cpp b/src/binary_annotator.cpp index 98a8648662..bcb0b6e929 100644 --- a/src/binary_annotator.cpp +++ b/src/binary_annotator.cpp @@ -753,8 +753,16 @@ void BinaryAnnotator::BuildTable(const uint64_t table_offset, switch (field->type()->base_type()) { case reflection::BaseType::Obj: { - const reflection::Object* next_object = - schema_->objects()->Get(field->type()->index()); + const reflection::Object* next_object = GetObject(field); + if (next_object == nullptr) { + // Field references a non-existent Object in the schema. Treat as a + // generic offset field annotation. + offset_field_comment.default_value = "(unknown)"; + regions.push_back(MakeBinaryRegion(field_offset, length, region_type, + 0, offset_of_next_item, + offset_field_comment)); + break; + } if (next_object->is_struct()) { // Structs are stored inline. @@ -911,9 +919,14 @@ uint64_t BinaryAnnotator::BuildStruct(const uint64_t struct_offset, offset += type_size; } else if (field->type()->base_type() == reflection::BaseType::Obj) { // Structs are stored inline, even when nested. + const reflection::Object* nested = GetObject(field); + if (nested == nullptr) { + // OOB Object index; skip further traversal. + return; + } offset = BuildStruct(offset, regions, referring_field_name + "." + field->name()->str(), - schema_->objects()->Get(field->type()->index())); + nested); } else if (field->type()->base_type() == reflection::BaseType::Array) { const bool is_scalar = IsScalar(field->type()->element()); const uint64_t type_size = GetTypeSize(field->type()->element()); @@ -961,10 +974,15 @@ uint64_t BinaryAnnotator::BuildStruct(const uint64_t struct_offset, // TODO(dbaileychess): This works, but the comments on the fields lose // some context. Need to figure a way how to plumb the nested arrays // comments together that isn't too confusing. + const reflection::Object* nested = GetObject(field); + if (nested == nullptr) { + // OOB Object index; skip further traversal. + break; + } offset = BuildStruct(offset, regions, referring_field_name + "." + field->name()->str(), - schema_->objects()->Get(field->type()->index())); + nested); } } } @@ -1131,8 +1149,11 @@ void BinaryAnnotator::BuildVector( switch (field->type()->element()) { case reflection::BaseType::Obj: { - const reflection::Object* object = - schema_->objects()->Get(field->type()->index()); + const reflection::Object* object = GetObject(field); + if (object == nullptr) { + // OOB Object index; cannot annotate further. + return; + } if (object->is_struct()) { // Vector of structs @@ -1408,8 +1429,11 @@ void BinaryAnnotator::BuildVector( std::string BinaryAnnotator::BuildUnion(const uint64_t union_offset, const uint8_t realized_type, const reflection::Field* const field) { - const reflection::Enum* next_enum = - schema_->enums()->Get(field->type()->index()); + const reflection::Enum* next_enum = GetEnum(field); + if (next_enum == nullptr) { + // OOB Enum index; cannot annotate further. + return ""; + } const reflection::EnumVal* enum_val = next_enum->values()->Get(realized_type); @@ -1420,8 +1444,13 @@ std::string BinaryAnnotator::BuildUnion(const uint64_t union_offset, const reflection::Type* union_type = enum_val->union_type(); if (union_type->base_type() == reflection::BaseType::Obj) { + const int32_t union_index = union_type->index(); + if (union_index < 0 || + static_cast(union_index) >= schema_->objects()->size()) { + return enum_val->name()->c_str(); + } const reflection::Object* object = - schema_->objects()->Get(union_type->index()); + schema_->objects()->Get(static_cast(union_index)); if (object->is_struct()) { // Union of vectors point to a new Binary section diff --git a/src/binary_annotator.h b/src/binary_annotator.h index d1f1af2e1a..069c592828 100644 --- a/src/binary_annotator.h +++ b/src/binary_annotator.h @@ -391,7 +391,9 @@ class BinaryAnnotator { bool IsInlineField(const reflection::Field* const field) { if (field->type()->base_type() == reflection::BaseType::Obj) { - return schema_->objects()->Get(field->type()->index())->is_struct(); + const reflection::Object* object = GetObject(field); + if (object == nullptr) return false; + return object->is_struct(); } return IsScalar(field->type()->base_type()); } @@ -426,6 +428,29 @@ class BinaryAnnotator { return value < enum_def->values()->size(); } + // Returns the Object referenced by `field`, or nullptr if the index is out of + // bounds. The .bfbs file is user-controlled; without this check the + // BinaryAnnotator dereferences out-of-bounds memory. + const reflection::Object* GetObject(const reflection::Field* const field) { + const int32_t index = field->type()->index(); + if (index < 0 || + static_cast(index) >= schema_->objects()->size()) { + return nullptr; + } + return schema_->objects()->Get(static_cast(index)); + } + + // Returns the Enum referenced by `field`, or nullptr if the index is out of + // bounds. + const reflection::Enum* GetEnum(const reflection::Field* const field) { + const int32_t index = field->type()->index(); + if (index < 0 || + static_cast(index) >= schema_->enums()->size()) { + return nullptr; + } + return schema_->enums()->Get(static_cast(index)); + } + uint64_t GetElementSize(const reflection::Field* const field) { if (IsScalar(field->type()->element())) { return GetTypeSize(field->type()->element()); @@ -433,7 +458,8 @@ class BinaryAnnotator { switch (field->type()->element()) { case reflection::BaseType::Obj: { - auto obj = schema_->objects()->Get(field->type()->index()); + const reflection::Object* obj = GetObject(field); + if (obj == nullptr) return sizeof(uint32_t); return obj->is_struct() ? obj->bytesize() : sizeof(uint32_t); } default: