From deb5f087b4352ca42d4f21218cbd52adc440d05f Mon Sep 17 00:00:00 2001 From: Mundur <150439604+M0nd0R@users.noreply.github.com> Date: Tue, 14 Jul 2026 09:57:21 +0800 Subject: [PATCH] Harden BFBS reflection verification Reject malformed BFBS metadata before it reaches reflection helpers and add regression coverage for invalid field ids, type indices, struct layouts, and union enum ordering. Tested with: - cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DFLATBUFFERS_BUILD_TESTS=ON - ninja -C build flattests - build/flattests --- include/flatbuffers/reflection.h | 47 +++- include/flatbuffers/reflection_generated.h | 228 +++++++++++++++-- src/bfbs_gen.h | 13 +- src/binary_annotator.cpp | 5 +- src/binary_annotator.h | 2 +- src/idl_parser.cpp | 62 +++-- src/reflection.cpp | 174 +++++++++++-- tests/reflection_test.cpp | 271 +++++++++++++++++++++ tests/reflection_test.h | 4 + tests/test.cpp | 4 + 10 files changed, 734 insertions(+), 76 deletions(-) diff --git a/include/flatbuffers/reflection.h b/include/flatbuffers/reflection.h index bc8c2a928..9d6c87a03 100644 --- a/include/flatbuffers/reflection.h +++ b/include/flatbuffers/reflection.h @@ -75,16 +75,37 @@ static_assert(sizeof(kBaseTypeSize) / sizeof(size_t) == // Size of a basic type, don't use with structs. constexpr size_t GetTypeSize(reflection::BaseType base_type) { - return kBaseTypeSize[base_type]; + const auto base_type_value = static_cast(base_type); + return base_type_value <= static_cast(reflection::MaxBaseType) + ? kBaseTypeSize[base_type_value] + : 0; +} + +inline const reflection::Object* GetTypeObjectByIndex( + const reflection::Schema& schema, int type_index) { + const auto* objects = schema.objects(); + return type_index >= 0 && objects && + static_cast(type_index) < objects->size() + ? objects->Get(static_cast(type_index)) + : nullptr; +} + +inline const reflection::Enum* GetTypeEnumByIndex( + const reflection::Schema& schema, int type_index) { + const auto* enums = schema.enums(); + return type_index >= 0 && enums && + static_cast(type_index) < enums->size() + ? enums->Get(static_cast(type_index)) + : nullptr; } // Same as above, but now correctly returns the size of a struct if // the field (or vector element) is a struct. inline size_t GetTypeSizeInline(reflection::BaseType base_type, int type_index, const reflection::Schema& schema) { - if (base_type == reflection::Obj && - schema.objects()->Get(type_index)->is_struct()) { - return schema.objects()->Get(type_index)->bytesize(); + const auto* object = GetTypeObjectByIndex(schema, type_index); + if (base_type == reflection::Obj && object && object->is_struct()) { + return object->bytesize(); } else { return GetTypeSize(base_type); } @@ -425,17 +446,27 @@ pointer_inside_vector piv(T* ptr, std::vector& vec) { constexpr const char* UnionTypeFieldSuffix() { return "_type"; } // Helper to figure out the actual table type a union refers to. -inline const reflection::Object& GetUnionType( +inline const reflection::Object* GetUnionTypeObject( const reflection::Schema& schema, const reflection::Object& parent, const reflection::Field& unionfield, const Table& table) { - auto enumdef = schema.enums()->Get(unionfield.type()->index()); + auto enumdef = GetTypeEnumByIndex(schema, unionfield.type()->index()); + if (!enumdef) return nullptr; // TODO: this is clumsy and slow, but no other way to find it? auto type_field = parent.fields()->LookupByKey( (unionfield.name()->str() + UnionTypeFieldSuffix()).c_str()); - FLATBUFFERS_ASSERT(type_field); + if (!type_field) return nullptr; auto union_type = GetFieldI(table, *type_field); auto enumval = enumdef->values()->LookupByKey(union_type); - return *schema.objects()->Get(enumval->union_type()->index()); + if (!enumval || !enumval->union_type()) return nullptr; + return GetTypeObjectByIndex(schema, enumval->union_type()->index()); +} + +inline const reflection::Object& GetUnionType( + const reflection::Schema& schema, const reflection::Object& parent, + const reflection::Field& unionfield, const Table& table) { + auto object = GetUnionTypeObject(schema, parent, unionfield, table); + FLATBUFFERS_ASSERT(object); + return *object; } // Changes the contents of a string inside a FlatBuffer. FlatBuffer must diff --git a/include/flatbuffers/reflection_generated.h b/include/flatbuffers/reflection_generated.h index 32ee1aa6b..066b5a442 100644 --- a/include/flatbuffers/reflection_generated.h +++ b/include/flatbuffers/reflection_generated.h @@ -1371,27 +1371,213 @@ struct Schema FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table { } template bool Verify(::flatbuffers::VerifierTemplate &verifier) const { - return VerifyTableStart(verifier) && - VerifyOffsetRequired(verifier, VT_OBJECTS) && - verifier.VerifyVector(objects()) && - verifier.VerifyVectorOfTables(objects()) && - VerifyOffsetRequired(verifier, VT_ENUMS) && - verifier.VerifyVector(enums()) && - verifier.VerifyVectorOfTables(enums()) && - VerifyOffset(verifier, VT_FILE_IDENT) && - verifier.VerifyString(file_ident()) && - VerifyOffset(verifier, VT_FILE_EXT) && - verifier.VerifyString(file_ext()) && - VerifyOffset(verifier, VT_ROOT_TABLE) && - verifier.VerifyTable(root_table()) && - VerifyOffset(verifier, VT_SERVICES) && - verifier.VerifyVector(services()) && - verifier.VerifyVectorOfTables(services()) && - VerifyField(verifier, VT_ADVANCED_FEATURES, 8) && - VerifyOffset(verifier, VT_FBS_FILES) && - verifier.VerifyVector(fbs_files()) && - verifier.VerifyVectorOfTables(fbs_files()) && - verifier.EndTable(); + if (!(VerifyTableStart(verifier) && + VerifyOffsetRequired(verifier, VT_OBJECTS) && + verifier.VerifyVector(objects()) && + verifier.VerifyVectorOfTables(objects()) && + VerifyOffsetRequired(verifier, VT_ENUMS) && + verifier.VerifyVector(enums()) && + verifier.VerifyVectorOfTables(enums()) && + VerifyOffset(verifier, VT_FILE_IDENT) && + verifier.VerifyString(file_ident()) && + VerifyOffset(verifier, VT_FILE_EXT) && + verifier.VerifyString(file_ext()) && + VerifyOffset(verifier, VT_ROOT_TABLE) && + verifier.VerifyTable(root_table()) && + VerifyOffset(verifier, VT_SERVICES) && + verifier.VerifyVector(services()) && + verifier.VerifyVectorOfTables(services()) && + VerifyField(verifier, VT_ADVANCED_FEATURES, 8) && + VerifyOffset(verifier, VT_FBS_FILES) && + verifier.VerifyVector(fbs_files()) && + verifier.VerifyVectorOfTables(fbs_files()) && + verifier.EndTable())) { + return false; + } + + const auto object_count = objects()->size(); + const auto enum_count = enums()->size(); + auto get_object = [&](int32_t index) -> const reflection::Object * { + return index >= 0 && + static_cast<::flatbuffers::uoffset_t>(index) < object_count + ? objects()->Get(static_cast<::flatbuffers::uoffset_t>(index)) + : nullptr; + }; + auto get_enum = [&](int32_t index) -> const reflection::Enum * { + return index >= 0 && + static_cast<::flatbuffers::uoffset_t>(index) < enum_count + ? enums()->Get(static_cast<::flatbuffers::uoffset_t>(index)) + : nullptr; + }; + auto is_power_of_two = [](uint64_t value) { + return value != 0 && (value & (value - 1)) == 0; + }; + auto scalar_size = [](reflection::BaseType type) -> uint64_t { + switch (type) { + case reflection::UType: + case reflection::Bool: + case reflection::Byte: + case reflection::UByte: return 1; + case reflection::Short: + case reflection::UShort: return 2; + case reflection::Int: + case reflection::UInt: + case reflection::Float: return 4; + case reflection::Long: + case reflection::ULong: + case reflection::Double: return 8; + default: return 0; + } + }; + auto inline_layout = [&](const reflection::Type *type, uint64_t *size, + uint64_t *align) { + const auto scalar = scalar_size(type->base_type()); + if (scalar != 0) { + *size = scalar; + *align = scalar; + return true; + } + + if (type->base_type() == reflection::Obj) { + const auto *object = get_object(type->index()); + if (!(object && object->is_struct())) return false; + if (!verifier.Check(object->minalign() > 0 && + object->bytesize() >= 0)) { + return false; + } + const auto object_align = static_cast(object->minalign()); + const auto object_size = static_cast(object->bytesize()); + if (!verifier.Check(is_power_of_two(object_align) && + object_size % object_align == 0)) { + return false; + } + *size = object_size; + *align = object_align; + return true; + } + + if (type->base_type() != reflection::Array || + !verifier.Check(type->fixed_length() > 0)) { + return false; + } + + uint64_t element_size = scalar_size(type->element()); + uint64_t element_align = element_size; + if (element_size == 0) { + if (type->element() != reflection::Obj) return false; + const auto *object = get_object(type->index()); + if (!(object && object->is_struct())) return false; + if (!verifier.Check(object->minalign() > 0 && + object->bytesize() >= 0)) { + return false; + } + element_align = static_cast(object->minalign()); + element_size = static_cast(object->bytesize()); + if (!verifier.Check(is_power_of_two(element_align) && + element_size % element_align == 0)) { + return false; + } + } + + *size = element_size * static_cast(type->fixed_length()); + *align = element_align; + return true; + }; + auto verify_type = [&](const reflection::Type *type) { + const auto base_type = static_cast(type->base_type()); + const auto element = static_cast(type->element()); + if (!verifier.Check(base_type >= reflection::None && + base_type <= reflection::MaxBaseType && + element >= reflection::None && + element <= reflection::MaxBaseType)) { + return false; + } + + const auto index = type->index(); + switch (type->base_type()) { + case reflection::Obj: + return verifier.Check(get_object(index) != nullptr); + case reflection::Union: + return verifier.Check(get_enum(index) != nullptr); + case reflection::Vector: + case reflection::Vector64: + case reflection::Array: + if (type->element() == reflection::Obj) { + return verifier.Check(get_object(index) != nullptr); + } + if (type->element() == reflection::Union) { + return verifier.Check(get_enum(index) != nullptr); + } + return true; + default: return true; + } + }; + + for (::flatbuffers::uoffset_t i = 0; i < enums()->size(); ++i) { + const auto *enum_def = enums()->Get(i); + if (!verify_type(enum_def->underlying_type())) return false; + for (::flatbuffers::uoffset_t j = 0; j < enum_def->values()->size(); + ++j) { + const auto *enum_val = enum_def->values()->Get(j); + if (enum_def->is_union() && j > 0 && + !verifier.Check(enum_def->values()->Get(j - 1)->value() < + enum_val->value())) { + return false; + } + const auto *union_type = enum_val->union_type(); + if (union_type && !verify_type(union_type)) return false; + } + } + + for (::flatbuffers::uoffset_t i = 0; i < objects()->size(); ++i) { + const auto *object = objects()->Get(i); + const auto *fields = object->fields(); + if (object->is_struct()) { + if (!verifier.Check(object->minalign() > 0 && + object->bytesize() >= 0)) { + return false; + } + const auto object_align = static_cast(object->minalign()); + const auto object_size = static_cast(object->bytesize()); + if (!verifier.Check(is_power_of_two(object_align) && + object_size % object_align == 0)) { + return false; + } + } + std::vector seen(fields->size(), 0); + for (::flatbuffers::uoffset_t j = 0; j < fields->size(); ++j) { + const auto *field = fields->Get(j); + if (!verify_type(field->type())) return false; + const auto field_id = field->id(); + if (!verifier.Check(field_id < fields->size())) return false; + if (!verifier.Check(seen[field_id] == 0)) return false; + seen[field_id] = 1; + if (object->is_struct()) { + uint64_t field_size = 0; + uint64_t field_align = 0; + if (!inline_layout(field->type(), &field_size, &field_align)) { + return false; + } + const auto field_offset = static_cast(field->offset()); + const auto field_padding = static_cast(field->padding()); + const auto object_size = static_cast(object->bytesize()); + if (!verifier.Check(field_align > 0 && + field_offset % field_align == 0)) { + return false; + } + if (!verifier.Check(field_offset <= object_size && + field_size <= object_size - field_offset)) { + return false; + } + if (!verifier.Check( + field_padding <= object_size - field_offset - field_size)) { + return false; + } + } + } + } + + return true; } }; diff --git a/src/bfbs_gen.h b/src/bfbs_gen.h index afae9c45f..8c4a72bfa 100644 --- a/src/bfbs_gen.h +++ b/src/bfbs_gen.h @@ -66,13 +66,20 @@ static void ForAllDocumentation( // Maps the field index into object->fields() to the field's ID (the ith element // in the return vector). static std::vector FieldIdToIndex(const reflection::Object* object) { - std::vector field_index_by_id; - field_index_by_id.resize(object->fields()->size()); + const auto field_count = object->fields()->size(); + std::vector field_index_by_id(field_count); + for (uint32_t i = 0; i < field_count; ++i) { field_index_by_id[i] = i; } + + std::vector seen(field_count, 0); // Create the mapping of field ID to the index into the vector. - for (uint32_t i = 0; i < object->fields()->size(); ++i) { + for (uint32_t i = 0; i < field_count; ++i) { auto field = object->fields()->Get(i); + if (field->id() >= field_count || seen[field->id()]) { + return field_index_by_id; + } field_index_by_id[field->id()] = i; + seen[field->id()] = 1; } return field_index_by_id; diff --git a/src/binary_annotator.cpp b/src/binary_annotator.cpp index 98a864866..14cb4897f 100644 --- a/src/binary_annotator.cpp +++ b/src/binary_annotator.cpp @@ -1411,13 +1411,16 @@ std::string BinaryAnnotator::BuildUnion(const uint64_t union_offset, const reflection::Enum* next_enum = schema_->enums()->Get(field->type()->index()); - const reflection::EnumVal* enum_val = next_enum->values()->Get(realized_type); + const reflection::EnumVal* enum_val = + next_enum->values()->LookupByKey(realized_type); + if (!enum_val) { return ""; } if (ContainsSection(union_offset)) { return enum_val->name()->c_str(); } const reflection::Type* union_type = enum_val->union_type(); + if (!union_type) { return ""; } if (union_type->base_type() == reflection::BaseType::Obj) { const reflection::Object* object = diff --git a/src/binary_annotator.h b/src/binary_annotator.h index d1f1af2e1..810c0b75a 100644 --- a/src/binary_annotator.h +++ b/src/binary_annotator.h @@ -423,7 +423,7 @@ class BinaryAnnotator { return false; } - return value < enum_def->values()->size(); + return enum_def->values()->LookupByKey(value) != nullptr; } uint64_t GetElementSize(const reflection::Field* const field) { diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index b1bdffa01..3427c367e 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -4132,24 +4132,50 @@ bool StructDef::Deserialize(Parser& parser, const reflection::Object* object) { sortbysize = attributes.Lookup("original_order") == nullptr && !fixed; const auto& of = *(object->fields()); auto indexes = std::vector(of.size()); + auto seen = std::vector(of.size(), 0); for (uoffset_t i = 0; i < of.size(); i++) { - uint16_t field_id = of.Get(i)->id(); - if (field_id >= of.size()) { - parser.error_ = "Field ID " + std::to_string(field_id) + - " exceeds field count " + std::to_string(of.size()); - return false; + uint16_t field_id = of.Get(i)->id(); + if (field_id >= of.size()) { + parser.error_ = "Field ID " + std::to_string(field_id) + + " exceeds field count " + std::to_string(of.size()); + return false; + } + if (seen[field_id]) { + parser.error_ = "Duplicate field ID " + std::to_string(field_id); + return false; + } + seen[field_id] = 1; + indexes[field_id] = i; } - indexes[field_id] = i; -} size_t tmp_struct_size = 0; for (size_t i = 0; i < indexes.size(); i++) { auto field = of.Get(indexes[i]); auto field_def = new FieldDef(); - if (!field_def->Deserialize(parser, field) || - fields.Add(field_def->name, field_def)) { + if (!field_def->Deserialize(parser, field)) { delete field_def; return false; } + if (fixed) { + // Recompute padding since that's currently not serialized. + auto size = InlineSize(field_def->value.type); + auto next_field = + i + 1 < indexes.size() ? of.Get(indexes[i + 1]) : nullptr; + tmp_struct_size += size; + if (next_field) { + const auto next_offset = next_field->offset(); + const auto field_end = field_def->value.offset + size; + if (next_offset < field_end) { + parser.error_ = "Invalid struct layout for field `" + + field_def->name + "`"; + delete field_def; + return false; + } + field_def->padding = next_offset - field_end; + } else { + field_def->padding = PaddingBytes(tmp_struct_size, minalign); + } + tmp_struct_size += field_def->padding; + } if (field_def->key) { if (has_key) { // only one field may be set as key @@ -4158,19 +4184,15 @@ bool StructDef::Deserialize(Parser& parser, const reflection::Object* object) { } has_key = true; } - if (fixed) { - // Recompute padding since that's currently not serialized. - auto size = InlineSize(field_def->value.type); - auto next_field = - i + 1 < indexes.size() ? of.Get(indexes[i + 1]) : nullptr; - tmp_struct_size += size; - field_def->padding = - next_field ? (next_field->offset() - field_def->value.offset) - size - : PaddingBytes(tmp_struct_size, minalign); - tmp_struct_size += field_def->padding; + if (fields.Add(field_def->name, field_def)) { + delete field_def; + return false; } } - FLATBUFFERS_ASSERT(static_cast(tmp_struct_size) == object->bytesize()); + if (fixed && static_cast(tmp_struct_size) != object->bytesize()) { + parser.error_ = "Struct size mismatch for `" + name + "`"; + return false; + } return true; } diff --git a/src/reflection.cpp b/src/reflection.cpp index 268d7d851..483a08681 100644 --- a/src/reflection.cpp +++ b/src/reflection.cpp @@ -32,7 +32,107 @@ static void CopyInline(FlatBufferBuilder& fbb, fbb.TrackField(fielddef.offset(), fbb.GetSize()); } +static bool IsPowerOfTwo(size_t value) { + return value != 0 && (value & (value - 1)) == 0; +} + +static size_t GetScalarTypeSize(const reflection::BaseType base_type) { + switch (base_type) { + case reflection::UType: + case reflection::Bool: + case reflection::Byte: + case reflection::UByte: + case reflection::Short: + case reflection::UShort: + case reflection::Int: + case reflection::UInt: + case reflection::Long: + case reflection::ULong: + case reflection::Float: + case reflection::Double: return GetTypeSize(base_type); + default: return 0; + } +} + +static bool GetInlineSizeAndAlignment(const reflection::Schema& schema, + const reflection::Type& type, + size_t* size, size_t* align) { + const auto scalar_size = GetScalarTypeSize(type.base_type()); + if (scalar_size != 0) { + *size = scalar_size; + *align = scalar_size; + return true; + } + + if (type.base_type() == reflection::Obj) { + auto* object = GetTypeObjectByIndex(schema, type.index()); + if (!(object && object->is_struct()) || object->minalign() <= 0 || + object->bytesize() < 0) { + return false; + } + *size = static_cast(object->bytesize()); + *align = static_cast(object->minalign()); + return IsPowerOfTwo(*align) && (*size % *align) == 0; + } + + if (type.base_type() != reflection::Array || type.fixed_length() == 0) { + return false; + } + + const auto element_scalar_size = GetScalarTypeSize(type.element()); + if (element_scalar_size != 0) { + *size = element_scalar_size * type.fixed_length(); + *align = element_scalar_size; + return true; + } + + if (type.element() != reflection::Obj) return false; + auto* object = GetTypeObjectByIndex(schema, type.index()); + if (!(object && object->is_struct()) || object->minalign() <= 0 || + object->bytesize() < 0) { + return false; + } + *size = static_cast(object->bytesize()) * type.fixed_length(); + *align = static_cast(object->minalign()); + return IsPowerOfTwo(*align) && + (static_cast(object->bytesize()) % *align) == 0; +} + +static bool VerifyStructDef(const reflection::Schema& schema, + const reflection::Object& obj) { + if (!obj.is_struct() || obj.minalign() <= 0 || obj.bytesize() < 0) { + return false; + } + const auto object_align = static_cast(obj.minalign()); + const auto object_size = static_cast(obj.bytesize()); + if (!IsPowerOfTwo(object_align) || (object_size % object_align) != 0) { + return false; + } + + for (uoffset_t i = 0; i < obj.fields()->size(); ++i) { + auto* field = obj.fields()->Get(i); + size_t field_size = 0; + size_t field_align = 0; + if (!GetInlineSizeAndAlignment(schema, *field->type(), &field_size, + &field_align) || + field_align == 0) { + return false; + } + + const auto field_offset = static_cast(field->offset()); + const auto field_padding = static_cast(field->padding()); + if ((field_offset % field_align) != 0 || field_offset > object_size || + field_size > object_size - field_offset || + field_padding > object_size - field_offset - field_size) { + return false; + } + } + + return true; +} + static bool VerifyStruct(flatbuffers::Verifier& v, + const reflection::Schema& schema, const flatbuffers::Table& parent_table, voffset_t field_offset, const reflection::Object& obj, bool required) { @@ -42,11 +142,13 @@ static bool VerifyStruct(flatbuffers::Verifier& v, } return !offset || + (VerifyStructDef(schema, obj) && v.VerifyFieldStruct(reinterpret_cast(&parent_table), - offset, obj.bytesize(), obj.minalign()); + offset, obj.bytesize(), obj.minalign())); } static bool VerifyVectorOfStructs(flatbuffers::Verifier& v, + const reflection::Schema& schema, const flatbuffers::Table& parent_table, voffset_t field_offset, const reflection::Object& obj, @@ -56,7 +158,8 @@ static bool VerifyVectorOfStructs(flatbuffers::Verifier& v, return false; } - return !p || v.VerifyVectorOrString(p, obj.bytesize()); + return !p || (VerifyStructDef(schema, obj) && + v.VerifyVectorOrString(p, obj.bytesize())); } // forward declare to resolve cyclic deps between VerifyObject and VerifyVector @@ -70,12 +173,15 @@ static bool VerifyUnion(flatbuffers::Verifier& v, const uint8_t* elem, const reflection::Field& union_field) { if (!utype) return true; // Not present. - auto fb_enum = schema.enums()->Get(union_field.type()->index()); - if (utype >= fb_enum->values()->size()) return false; - auto elem_type = fb_enum->values()->Get(utype)->union_type(); + auto fb_enum = GetTypeEnumByIndex(schema, union_field.type()->index()); + if (!fb_enum) return false; + auto elem_val = fb_enum->values()->LookupByKey(utype); + if (!elem_val || !elem_val->union_type()) return false; + auto elem_type = elem_val->union_type(); switch (elem_type->base_type()) { case reflection::Obj: { - auto elem_obj = schema.objects()->Get(elem_type->index()); + auto elem_obj = GetTypeObjectByIndex(schema, elem_type->index()); + if (!elem_obj) return false; if (elem_obj->is_struct()) { return v.VerifyFromPointer(elem, elem_obj->bytesize()); } else { @@ -130,9 +236,10 @@ static bool VerifyVector(flatbuffers::Verifier& v, } } case reflection::Obj: { - auto obj = schema.objects()->Get(vec_field.type()->index()); + auto obj = GetTypeObjectByIndex(schema, vec_field.type()->index()); + if (!obj) return false; if (obj->is_struct()) { - return VerifyVectorOfStructs(v, table, vec_field.offset(), *obj, + return VerifyVectorOfStructs(v, schema, table, vec_field.offset(), *obj, vec_field.required()); } else { auto vec = @@ -233,9 +340,11 @@ static bool VerifyObject(flatbuffers::Verifier& v, if (!VerifyVector(v, schema, *table, *field_def)) return false; break; case reflection::Obj: { - auto child_obj = schema.objects()->Get(field_def->type()->index()); + auto child_obj = + GetTypeObjectByIndex(schema, field_def->type()->index()); + if (!child_obj) return false; if (child_obj->is_struct()) { - if (!VerifyStruct(v, *table, field_def->offset(), *child_obj, + if (!VerifyStruct(v, schema, *table, field_def->offset(), *child_obj, field_def->required())) { return false; } @@ -378,18 +487,28 @@ std::string GetAnyValueS(reflection::BaseType type, const uint8_t* data, void ForAllFields(const reflection::Object* object, bool reverse, std::function func) { - std::vector field_to_id_map; - field_to_id_map.resize(object->fields()->size()); + const auto field_count = object->fields()->size(); + std::vector field_to_id_map(field_count); + for (uint32_t i = 0; i < field_count; ++i) { field_to_id_map[i] = i; } + + std::vector seen(field_count, 0); + bool use_id_order = true; // Create the mapping of field ID to the index into the vector. - for (uint32_t i = 0; i < object->fields()->size(); ++i) { + for (uint32_t i = 0; i < field_count; ++i) { auto field = object->fields()->Get(i); + if (field->id() >= field_count || seen[field->id()]) { + use_id_order = false; + break; + } field_to_id_map[field->id()] = i; + seen[field->id()] = 1; } for (size_t i = 0; i < field_to_id_map.size(); ++i) { - func(object->fields()->Get( - field_to_id_map[reverse ? field_to_id_map.size() - (i + 1) : i])); + const auto index = reverse ? field_to_id_map.size() - (i + 1) : i; + const auto field_index = use_id_order ? field_to_id_map[index] : index; + func(object->fields()->Get(field_index)); } } @@ -564,8 +683,11 @@ class ResizeContext { break; } case reflection::Union: { - ResizeTable(GetUnionType(schema_, objectdef, fielddef, *table), - reinterpret_cast(ref)); + auto union_object = + GetUnionTypeObject(schema_, objectdef, fielddef, *table); + if (union_object) { + ResizeTable(*union_object, reinterpret_cast(ref)); + } break; } case reflection::String: @@ -669,8 +791,10 @@ Offset CopyTable(FlatBufferBuilder& fbb, // Skip if field is not present in the source. if (!table.CheckField(fielddef.offset())) continue; uoffset_t offset = 0; + bool has_offset_slot = false; switch (fielddef.type()->base_type()) { case reflection::String: { + has_offset_slot = true; offset = use_string_pooling ? fbb.CreateSharedString(GetFieldS(table, fielddef)).o : fbb.CreateString(GetFieldS(table, fielddef)).o; @@ -679,6 +803,7 @@ Offset CopyTable(FlatBufferBuilder& fbb, case reflection::Obj: { auto& subobjectdef = *schema.objects()->Get(fielddef.type()->index()); if (!subobjectdef.is_struct()) { + has_offset_slot = true; offset = CopyTable(fbb, schema, subobjectdef, *GetFieldT(table, fielddef), use_string_pooling) .o; @@ -686,13 +811,18 @@ Offset CopyTable(FlatBufferBuilder& fbb, break; } case reflection::Union: { - auto& subobjectdef = GetUnionType(schema, objectdef, fielddef, table); - offset = CopyTable(fbb, schema, subobjectdef, - *GetFieldT(table, fielddef), use_string_pooling) - .o; + has_offset_slot = true; + auto* subobjectdef = + GetUnionTypeObject(schema, objectdef, fielddef, table); + if (subobjectdef) { + offset = CopyTable(fbb, schema, *subobjectdef, + *GetFieldT(table, fielddef), use_string_pooling) + .o; + } break; } case reflection::Vector: { + has_offset_slot = true; auto vec = table.GetPointer>*>(fielddef.offset()); auto element_base_type = fielddef.type()->element(); @@ -740,7 +870,7 @@ Offset CopyTable(FlatBufferBuilder& fbb, default: // Scalars. break; } - if (offset) { + if (offset || has_offset_slot) { offsets.push_back(offset); } } diff --git a/tests/reflection_test.cpp b/tests/reflection_test.cpp index 9f80b936c..aebaf1956 100644 --- a/tests/reflection_test.cpp +++ b/tests/reflection_test.cpp @@ -1,5 +1,16 @@ #include "reflection_test.h" +#include +#include + +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-function" +#endif +#include "bfbs_gen.h" +#if defined(__clang__) +#pragma clang diagnostic pop +#endif #include "flatbuffers/minireflect.h" #include "flatbuffers/reflection.h" #include "flatbuffers/reflection_generated.h" @@ -14,6 +25,189 @@ namespace tests { using namespace MyGame::Example; +namespace { + +std::vector LoadMalformedFieldIdSchema( + const std::string& tests_data_path) { + std::string bfbsfile; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monster_test.bfbs").c_str(), + true, &bfbsfile), + true); + + std::vector bfbs(bfbsfile.begin(), bfbsfile.end()); + flatbuffers::Verifier verifier_before(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_before), true); + + auto* schema = reflection::GetSchema(bfbs.data()); + auto* root = const_cast(schema->root_table()); + TEST_NOTNULL(root); + TEST_ASSERT(root->fields() && root->fields()->size() > 0); + + auto* field0 = const_cast(root->fields()->Get(0)); + auto* field_table = reinterpret_cast(field0); + TEST_EQ(field_table->SetField(reflection::Field::VT_ID, + std::numeric_limits::max(), + 0), + true); + + flatbuffers::Verifier verifier_after(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_after), false); + TEST_EQ(root->fields()->Get(0)->id(), std::numeric_limits::max()); + return bfbs; +} + +std::vector LoadMalformedTypeIndexSchema( + const std::string& tests_data_path) { + std::string bfbsfile; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monster_test.bfbs").c_str(), + true, &bfbsfile), + true); + + std::vector bfbs(bfbsfile.begin(), bfbsfile.end()); + flatbuffers::Verifier verifier_before(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_before), true); + + auto* schema = reflection::GetSchema(bfbs.data()); + auto* root = const_cast(schema->root_table()); + auto* pos_field = + const_cast(root->fields()->LookupByKey("pos")); + TEST_NOTNULL(pos_field); + + auto* type_table = + reinterpret_cast(const_cast( + pos_field->type())); + TEST_EQ(type_table->SetField( + reflection::Type::VT_INDEX, + std::numeric_limits::max(), -1), + true); + + flatbuffers::Verifier verifier_after(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_after), false); + TEST_EQ(pos_field->type()->index(), std::numeric_limits::max()); + return bfbs; +} + +std::vector LoadMalformedStructLayoutSchema( + const std::string& tests_data_path) { + std::string bfbsfile; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monster_test.bfbs").c_str(), + true, &bfbsfile), + true); + + std::vector bfbs(bfbsfile.begin(), bfbsfile.end()); + flatbuffers::Verifier verifier_before(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_before), true); + + auto* schema = reflection::GetSchema(bfbs.data()); + auto* vec3 = const_cast( + schema->objects()->LookupByKey("MyGame.Example.Vec3")); + TEST_NOTNULL(vec3); + auto* vec3_table = reinterpret_cast(vec3); + TEST_EQ(vec3_table->SetField(reflection::Object::VT_BYTESIZE, 1, 0), + true); + + auto* z_field = const_cast(vec3->fields()->LookupByKey("z")); + TEST_NOTNULL(z_field); + auto* z_field_table = reinterpret_cast(z_field); + TEST_EQ(z_field_table->SetField( + reflection::Field::VT_OFFSET, + std::numeric_limits::max(), 0), + true); + + flatbuffers::Verifier verifier_after(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_after), false); + TEST_EQ(vec3->bytesize(), 1); + TEST_EQ(z_field->offset(), std::numeric_limits::max()); + return bfbs; +} + +std::vector LoadMalformedUnionEnumSchema( + const std::string& tests_data_path) { + std::string bfbsfile; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monster_test.bfbs").c_str(), + true, &bfbsfile), + true); + + std::vector bfbs(bfbsfile.begin(), bfbsfile.end()); + flatbuffers::Verifier verifier_before(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_before), true); + + auto* schema = reflection::GetSchema(bfbs.data()); + auto* root = schema->root_table(); + auto* test_field = root->fields()->LookupByKey("test"); + TEST_NOTNULL(test_field); + auto* any_enum = const_cast( + schema->enums()->Get(test_field->type()->index())); + TEST_NOTNULL(any_enum); + + auto* monster_val = + const_cast(any_enum->values()->Get(1)); + TEST_NOTNULL(monster_val); + auto* monster_val_table = reinterpret_cast(monster_val); + TEST_EQ(monster_val_table->SetField(reflection::EnumVal::VT_VALUE, + 100, 0), + true); + + flatbuffers::Verifier verifier_after(bfbs.data(), bfbs.size()); + TEST_EQ(reflection::VerifySchemaBuffer(verifier_after), false); + TEST_EQ(monster_val->value(), 100); + return bfbs; +} + +class FieldIdMappingBfbsGenerator final : public BaseBfbsGenerator { + public: + using BaseBfbsGenerator::GenerateCode; + + Status GenerateFromSchema(const reflection::Schema* schema, + const CodeGenOptions&) override { + visited_fields.clear(); + TEST_NOTNULL(schema); + TEST_NOTNULL(schema->root_table()); + ForAllFields(schema->root_table(), /*reverse=*/false, + [&](const reflection::Field* field) { + visited_fields.push_back(field->name()->str()); + }); + return OK; + } + + Status GenerateCode(const Parser&, const std::string&, + const std::string&) override { + return NOT_IMPLEMENTED; + } + + Status GenerateMakeRule(const Parser&, const std::string&, + const std::string&, std::string&) override { + return NOT_IMPLEMENTED; + } + + Status GenerateGrpcCode(const Parser&, const std::string&, + const std::string&) override { + return NOT_IMPLEMENTED; + } + + Status GenerateRootFile(const Parser&, const std::string&) override { + return NOT_IMPLEMENTED; + } + + bool IsSchemaOnly() const override { return true; } + + bool SupportsBfbsGeneration() const override { return true; } + + bool SupportsRootFileGeneration() const override { return false; } + + IDLOptions::Language Language() const override { return IDLOptions::kCpp; } + + std::string LanguageName() const override { return "TestBfbs"; } + + uint64_t SupportedAdvancedFeatures() const override { + return std::numeric_limits::max(); + } + + std::vector visited_fields; +}; + +} // namespace + void ReflectionTest(const std::string& tests_data_path, uint8_t* flatbuf, size_t length) { // Load a binary schema. @@ -335,6 +529,83 @@ void ForAllFieldsReverseTest(const std::string& tests_data_path) { } } +void MalformedBfbsFieldIdsTest(const std::string& tests_data_path) { + auto bfbs = LoadMalformedFieldIdSchema(tests_data_path); + auto* schema = reflection::GetSchema(bfbs.data()); + auto* root = schema->root_table(); + TEST_NOTNULL(root); + + std::vector reflected_fields; + flatbuffers::ForAllFields(root, /*reverse=*/false, + [&](const reflection::Field* field) { + reflected_fields.push_back(field->name()->str()); + }); + TEST_EQ(reflected_fields.size(), root->fields()->size()); + TEST_EQ_STR(reflected_fields.front().c_str(), + root->fields()->Get(0)->name()->c_str()); + + FieldIdMappingBfbsGenerator generator; + CodeGenOptions options; + TEST_EQ(generator.GenerateFromSchema(schema, options), + CodeGenerator::OK); + TEST_EQ(generator.visited_fields.size(), root->fields()->size()); + TEST_EQ_STR(generator.visited_fields.front().c_str(), + root->fields()->Get(0)->name()->c_str()); +} + +void MalformedBfbsTypeIndexTest(const std::string& tests_data_path) { + auto bfbs = LoadMalformedTypeIndexSchema(tests_data_path); + auto* schema = reflection::GetSchema(bfbs.data()); + + std::string binary_contents; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monsterdata_test.mon").c_str(), + true, &binary_contents), + true); + TEST_EQ(flatbuffers::Verify( + *schema, *schema->root_table(), + reinterpret_cast(binary_contents.data()), + binary_contents.size()), + false); +} + +void MalformedBfbsStructLayoutTest(const std::string& tests_data_path) { + auto bfbs = LoadMalformedStructLayoutSchema(tests_data_path); + auto* schema = reflection::GetSchema(bfbs.data()); + + std::string binary_contents; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monsterdata_test.mon").c_str(), + true, &binary_contents), + true); + TEST_EQ(flatbuffers::Verify( + *schema, *schema->root_table(), + reinterpret_cast(binary_contents.data()), + binary_contents.size()), + false); +} + +void MalformedBfbsUnionEnumTest(const std::string& tests_data_path) { + auto bfbs = LoadMalformedUnionEnumSchema(tests_data_path); + auto* schema = reflection::GetSchema(bfbs.data()); + + std::string binary_contents; + TEST_EQ(flatbuffers::LoadFile((tests_data_path + "monsterdata_test.mon").c_str(), + true, &binary_contents), + true); + auto* monster = flatbuffers::GetAnyRoot( + reinterpret_cast(binary_contents.data())); + TEST_EQ(flatbuffers::Verify( + *schema, *schema->root_table(), + reinterpret_cast(binary_contents.data()), + binary_contents.size()), + false); + + flatbuffers::FlatBufferBuilder builder; + auto copied = + flatbuffers::CopyTable(builder, *schema, *schema->root_table(), *monster, + /*use_string_pooling=*/false); + TEST_ASSERT(copied.o != 0); +} + void MiniReflectFlatBuffersTest(uint8_t* flatbuf) { auto s = flatbuffers::FlatBufferToString(flatbuf, Monster::MiniReflectTypeTable()); diff --git a/tests/reflection_test.h b/tests/reflection_test.h index da6fb1ff6..4a74d04d6 100644 --- a/tests/reflection_test.h +++ b/tests/reflection_test.h @@ -11,6 +11,10 @@ namespace tests { void ReflectionTest(const std::string& tests_data_path, uint8_t* flatbuf, size_t length); void ForAllFieldsReverseTest(const std::string& tests_data_path); +void MalformedBfbsFieldIdsTest(const std::string& tests_data_path); +void MalformedBfbsTypeIndexTest(const std::string& tests_data_path); +void MalformedBfbsStructLayoutTest(const std::string& tests_data_path); +void MalformedBfbsUnionEnumTest(const std::string& tests_data_path); void MiniReflectFixedLengthArrayTest(); void MiniReflectFlatBuffersTest(uint8_t* flatbuf); diff --git a/tests/test.cpp b/tests/test.cpp index 5a43546f5..b5a9ff1ae 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -1775,6 +1775,10 @@ int FlatBufferTests(const std::string& tests_data_path) { FixedLengthArrayJsonTest(tests_data_path, true); ReflectionTest(tests_data_path, flatbuf.data(), flatbuf.size()); ForAllFieldsReverseTest(tests_data_path); + MalformedBfbsFieldIdsTest(tests_data_path); + MalformedBfbsTypeIndexTest(tests_data_path); + MalformedBfbsStructLayoutTest(tests_data_path); + MalformedBfbsUnionEnumTest(tests_data_path); ParseProtoTest(tests_data_path); EvolutionTest(tests_data_path); UnionDeprecationTest(tests_data_path);