From 8b92122f33c2e2aa07e335341503ce19b4989abb Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Wed, 26 Apr 2017 14:26:18 -0700 Subject: [PATCH] Made the verifier catch zero-offsets. Zero offsets are non-sensical in FlatBuffers (since offsets are relative to themselves) but were allowed by the verifier. This could cause buffers made up of all zeroes to be interpreted as correct buffers with an empty root object. Generally, not allowing such offsets will make the verifier more likely to catch problems earlier. Change-Id: I54010bea29721b326ff8e5348fcd9fe78e5e7506 Tested: on Linux. --- include/flatbuffers/flatbuffers.h | 26 +++++++++-- include/flatbuffers/reflection_generated.h | 46 +++++++++---------- samples/monster_generated.h | 10 ++-- src/idl_gen_cpp.cpp | 9 +++- tests/monster_test_generated.h | 26 +++++------ .../namespace_test2_generated.h | 8 ++-- tests/union_vector/union_vector_generated.h | 6 +-- 7 files changed, 78 insertions(+), 53 deletions(-) diff --git a/include/flatbuffers/flatbuffers.h b/include/flatbuffers/flatbuffers.h index e38e55280..c42781619 100644 --- a/include/flatbuffers/flatbuffers.h +++ b/include/flatbuffers/flatbuffers.h @@ -1650,8 +1650,9 @@ class Verifier FLATBUFFERS_FINAL_CLASS { } // Call T::Verify, which must be in the generated code for this type. - return Verify(start) && - reinterpret_cast(start + ReadScalar(start))-> + auto o = VerifyOffset(start); + return o && + reinterpret_cast(start + o)-> Verify(*this) #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE && GetComputedSize() @@ -1674,6 +1675,13 @@ class Verifier FLATBUFFERS_FINAL_CLASS { VerifyBufferFromStart(identifier, buf_ + sizeof(uoffset_t)); } + uoffset_t VerifyOffset(const uint8_t *start) const { + if (!Verify(start)) return false; + auto o = ReadScalar(start); + Check(o != 0); + return o; + } + // Called at the start of a table to increase counters measuring data // structure depth and amount, and possibly bails out with false if // limits set by the constructor have been hit. Needs to be balanced @@ -1850,12 +1858,24 @@ class Table { // VerifyField for required fields. template bool VerifyFieldRequired(const Verifier &verifier, - voffset_t field) const { + voffset_t field) const { auto field_offset = GetOptionalFieldOffset(field); return verifier.Check(field_offset != 0) && verifier.Verify(data_ + field_offset); } + // Versions for offsets. + bool VerifyOffset(const Verifier &verifier, voffset_t field) const { + auto field_offset = GetOptionalFieldOffset(field); + return !field_offset || verifier.VerifyOffset(data_ + field_offset); + } + + bool VerifyOffsetRequired(const Verifier &verifier, voffset_t field) const { + auto field_offset = GetOptionalFieldOffset(field); + return verifier.Check(field_offset != 0) && + verifier.VerifyOffset(data_ + field_offset); + } + private: // private constructor & copy constructor: you obtain instances of this // class by pointing to existing data only diff --git a/include/flatbuffers/reflection_generated.h b/include/flatbuffers/reflection_generated.h index b76814e47..097084ba6 100644 --- a/include/flatbuffers/reflection_generated.h +++ b/include/flatbuffers/reflection_generated.h @@ -150,9 +150,9 @@ struct KeyValue FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyFieldRequired(verifier, VT_KEY) && + VerifyOffsetRequired(verifier, VT_KEY) && verifier.Verify(key()) && - VerifyField(verifier, VT_VALUE) && + VerifyOffset(verifier, VT_VALUE) && verifier.Verify(value()) && verifier.EndTable(); } @@ -234,12 +234,12 @@ struct EnumVal FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyFieldRequired(verifier, VT_NAME) && + VerifyOffsetRequired(verifier, VT_NAME) && verifier.Verify(name()) && VerifyField(verifier, VT_VALUE) && - VerifyField(verifier, VT_OBJECT) && + VerifyOffset(verifier, VT_OBJECT) && verifier.VerifyTable(object()) && - VerifyField(verifier, VT_UNION_TYPE) && + VerifyOffset(verifier, VT_UNION_TYPE) && verifier.VerifyTable(union_type()) && verifier.EndTable(); } @@ -336,18 +336,18 @@ struct Enum FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyFieldRequired(verifier, VT_NAME) && + VerifyOffsetRequired(verifier, VT_NAME) && verifier.Verify(name()) && - VerifyFieldRequired(verifier, VT_VALUES) && + VerifyOffsetRequired(verifier, VT_VALUES) && verifier.Verify(values()) && verifier.VerifyVectorOfTables(values()) && VerifyField(verifier, VT_IS_UNION) && - VerifyFieldRequired(verifier, VT_UNDERLYING_TYPE) && + VerifyOffsetRequired(verifier, VT_UNDERLYING_TYPE) && verifier.VerifyTable(underlying_type()) && - VerifyField(verifier, VT_ATTRIBUTES) && + VerifyOffset(verifier, VT_ATTRIBUTES) && verifier.Verify(attributes()) && verifier.VerifyVectorOfTables(attributes()) && - VerifyField(verifier, VT_DOCUMENTATION) && + VerifyOffset(verifier, VT_DOCUMENTATION) && verifier.Verify(documentation()) && verifier.VerifyVectorOfStrings(documentation()) && verifier.EndTable(); @@ -481,9 +481,9 @@ struct Field FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyFieldRequired(verifier, VT_NAME) && + VerifyOffsetRequired(verifier, VT_NAME) && verifier.Verify(name()) && - VerifyFieldRequired(verifier, VT_TYPE) && + VerifyOffsetRequired(verifier, VT_TYPE) && verifier.VerifyTable(type()) && VerifyField(verifier, VT_ID) && VerifyField(verifier, VT_OFFSET) && @@ -492,10 +492,10 @@ struct Field FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { VerifyField(verifier, VT_DEPRECATED) && VerifyField(verifier, VT_REQUIRED) && VerifyField(verifier, VT_KEY) && - VerifyField(verifier, VT_ATTRIBUTES) && + VerifyOffset(verifier, VT_ATTRIBUTES) && verifier.Verify(attributes()) && verifier.VerifyVectorOfTables(attributes()) && - VerifyField(verifier, VT_DOCUMENTATION) && + VerifyOffset(verifier, VT_DOCUMENTATION) && verifier.Verify(documentation()) && verifier.VerifyVectorOfStrings(documentation()) && verifier.EndTable(); @@ -647,18 +647,18 @@ struct Object FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyFieldRequired(verifier, VT_NAME) && + VerifyOffsetRequired(verifier, VT_NAME) && verifier.Verify(name()) && - VerifyFieldRequired(verifier, VT_FIELDS) && + VerifyOffsetRequired(verifier, VT_FIELDS) && verifier.Verify(fields()) && verifier.VerifyVectorOfTables(fields()) && VerifyField(verifier, VT_IS_STRUCT) && VerifyField(verifier, VT_MINALIGN) && VerifyField(verifier, VT_BYTESIZE) && - VerifyField(verifier, VT_ATTRIBUTES) && + VerifyOffset(verifier, VT_ATTRIBUTES) && verifier.Verify(attributes()) && verifier.VerifyVectorOfTables(attributes()) && - VerifyField(verifier, VT_DOCUMENTATION) && + VerifyOffset(verifier, VT_DOCUMENTATION) && verifier.Verify(documentation()) && verifier.VerifyVectorOfStrings(documentation()) && verifier.EndTable(); @@ -768,17 +768,17 @@ struct Schema FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyFieldRequired(verifier, VT_OBJECTS) && + VerifyOffsetRequired(verifier, VT_OBJECTS) && verifier.Verify(objects()) && verifier.VerifyVectorOfTables(objects()) && - VerifyFieldRequired(verifier, VT_ENUMS) && + VerifyOffsetRequired(verifier, VT_ENUMS) && verifier.Verify(enums()) && verifier.VerifyVectorOfTables(enums()) && - VerifyField(verifier, VT_FILE_IDENT) && + VerifyOffset(verifier, VT_FILE_IDENT) && verifier.Verify(file_ident()) && - VerifyField(verifier, VT_FILE_EXT) && + VerifyOffset(verifier, VT_FILE_EXT) && verifier.Verify(file_ext()) && - VerifyField(verifier, VT_ROOT_TABLE) && + VerifyOffset(verifier, VT_ROOT_TABLE) && verifier.VerifyTable(root_table()) && verifier.EndTable(); } diff --git a/samples/monster_generated.h b/samples/monster_generated.h index fb261849c..fff866e1f 100644 --- a/samples/monster_generated.h +++ b/samples/monster_generated.h @@ -239,16 +239,16 @@ struct Monster FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { VerifyField(verifier, VT_POS) && VerifyField(verifier, VT_MANA) && VerifyField(verifier, VT_HP) && - VerifyField(verifier, VT_NAME) && + VerifyOffset(verifier, VT_NAME) && verifier.Verify(name()) && - VerifyField(verifier, VT_INVENTORY) && + VerifyOffset(verifier, VT_INVENTORY) && verifier.Verify(inventory()) && VerifyField(verifier, VT_COLOR) && - VerifyField(verifier, VT_WEAPONS) && + VerifyOffset(verifier, VT_WEAPONS) && verifier.Verify(weapons()) && verifier.VerifyVectorOfTables(weapons()) && VerifyField(verifier, VT_EQUIPPED_TYPE) && - VerifyField(verifier, VT_EQUIPPED) && + VerifyOffset(verifier, VT_EQUIPPED) && VerifyEquipment(verifier, equipped(), equipped_type()) && verifier.EndTable(); } @@ -382,7 +382,7 @@ struct Weapon FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyField(verifier, VT_NAME) && + VerifyOffset(verifier, VT_NAME) && verifier.Verify(name()) && VerifyField(verifier, VT_DAMAGE) && verifier.EndTable(); diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index b0fd11d7d..2f97f83fd 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -1110,7 +1110,12 @@ class CppGenerator : public BaseGenerator { code_.SetValue("REQUIRED", field.required ? "Required" : ""); code_.SetValue("SIZE", GenTypeSize(field.value.type)); code_.SetValue("OFFSET", GenFieldOffsetName(field)); - code_ += "{{PRE}}VerifyField{{REQUIRED}}<{{SIZE}}>(verifier, {{OFFSET}})\\"; + if (IsScalar(field.value.type.base_type) || IsStruct(field.value.type)) { + code_ += + "{{PRE}}VerifyField{{REQUIRED}}<{{SIZE}}>(verifier, {{OFFSET}})\\"; + } else { + code_ += "{{PRE}}VerifyOffset{{REQUIRED}}(verifier, {{OFFSET}})\\"; + } switch (field.value.type.base_type) { case BASE_TYPE_UNION: { @@ -1764,7 +1769,7 @@ class CppGenerator : public BaseGenerator { code += "_fbb.CreateVectorOfNativeStructs<"; code += WrapInNameSpace(*vector_type.struct_def) + ">"; } else { - code += "_fbb.CreateVectorOfStructs"; + code += "_fbb.CreateVectorOfStructs"; } code += "(" + value + ")"; } else { diff --git a/tests/monster_test_generated.h b/tests/monster_test_generated.h index 10aecc809..fc43f5d43 100644 --- a/tests/monster_test_generated.h +++ b/tests/monster_test_generated.h @@ -433,7 +433,7 @@ struct Stat FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyField(verifier, VT_ID) && + VerifyOffset(verifier, VT_ID) && verifier.Verify(id()) && VerifyField(verifier, VT_VAL) && VerifyField(verifier, VT_COUNT) && @@ -778,27 +778,27 @@ struct Monster FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { VerifyField(verifier, VT_POS) && VerifyField(verifier, VT_MANA) && VerifyField(verifier, VT_HP) && - VerifyFieldRequired(verifier, VT_NAME) && + VerifyOffsetRequired(verifier, VT_NAME) && verifier.Verify(name()) && - VerifyField(verifier, VT_INVENTORY) && + VerifyOffset(verifier, VT_INVENTORY) && verifier.Verify(inventory()) && VerifyField(verifier, VT_COLOR) && VerifyField(verifier, VT_TEST_TYPE) && - VerifyField(verifier, VT_TEST) && + VerifyOffset(verifier, VT_TEST) && VerifyAny(verifier, test(), test_type()) && - VerifyField(verifier, VT_TEST4) && + VerifyOffset(verifier, VT_TEST4) && verifier.Verify(test4()) && - VerifyField(verifier, VT_TESTARRAYOFSTRING) && + VerifyOffset(verifier, VT_TESTARRAYOFSTRING) && verifier.Verify(testarrayofstring()) && verifier.VerifyVectorOfStrings(testarrayofstring()) && - VerifyField(verifier, VT_TESTARRAYOFTABLES) && + VerifyOffset(verifier, VT_TESTARRAYOFTABLES) && verifier.Verify(testarrayoftables()) && verifier.VerifyVectorOfTables(testarrayoftables()) && - VerifyField(verifier, VT_ENEMY) && + VerifyOffset(verifier, VT_ENEMY) && verifier.VerifyTable(enemy()) && - VerifyField(verifier, VT_TESTNESTEDFLATBUFFER) && + VerifyOffset(verifier, VT_TESTNESTEDFLATBUFFER) && verifier.Verify(testnestedflatbuffer()) && - VerifyField(verifier, VT_TESTEMPTY) && + VerifyOffset(verifier, VT_TESTEMPTY) && verifier.VerifyTable(testempty()) && VerifyField(verifier, VT_TESTBOOL) && VerifyField(verifier, VT_TESTHASHS32_FNV1) && @@ -809,15 +809,15 @@ struct Monster FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { VerifyField(verifier, VT_TESTHASHU32_FNV1A) && VerifyField(verifier, VT_TESTHASHS64_FNV1A) && VerifyField(verifier, VT_TESTHASHU64_FNV1A) && - VerifyField(verifier, VT_TESTARRAYOFBOOLS) && + VerifyOffset(verifier, VT_TESTARRAYOFBOOLS) && verifier.Verify(testarrayofbools()) && VerifyField(verifier, VT_TESTF) && VerifyField(verifier, VT_TESTF2) && VerifyField(verifier, VT_TESTF3) && - VerifyField(verifier, VT_TESTARRAYOFSTRING2) && + VerifyOffset(verifier, VT_TESTARRAYOFSTRING2) && verifier.Verify(testarrayofstring2()) && verifier.VerifyVectorOfStrings(testarrayofstring2()) && - VerifyField(verifier, VT_TESTARRAYOFSORTEDSTRUCT) && + VerifyOffset(verifier, VT_TESTARRAYOFSORTEDSTRUCT) && verifier.Verify(testarrayofsortedstruct()) && verifier.EndTable(); } diff --git a/tests/namespace_test/namespace_test2_generated.h b/tests/namespace_test/namespace_test2_generated.h index 4e2a622c9..6c1304793 100644 --- a/tests/namespace_test/namespace_test2_generated.h +++ b/tests/namespace_test/namespace_test2_generated.h @@ -50,7 +50,7 @@ struct TableInFirstNS FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyField(verifier, VT_FOO_TABLE) && + VerifyOffset(verifier, VT_FOO_TABLE) && verifier.VerifyTable(foo_table()) && VerifyField(verifier, VT_FOO_ENUM) && VerifyField(verifier, VT_FOO_STRUCT) && @@ -117,9 +117,9 @@ struct TableInC FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyField(verifier, VT_REFER_TO_A1) && + VerifyOffset(verifier, VT_REFER_TO_A1) && verifier.VerifyTable(refer_to_a1()) && - VerifyField(verifier, VT_REFER_TO_A2) && + VerifyOffset(verifier, VT_REFER_TO_A2) && verifier.VerifyTable(refer_to_a2()) && verifier.EndTable(); } @@ -172,7 +172,7 @@ struct SecondTableInA FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && - VerifyField(verifier, VT_REFER_TO_C) && + VerifyOffset(verifier, VT_REFER_TO_C) && verifier.VerifyTable(refer_to_c()) && verifier.EndTable(); } diff --git a/tests/union_vector/union_vector_generated.h b/tests/union_vector/union_vector_generated.h index 104c66589..c871a28a2 100644 --- a/tests/union_vector/union_vector_generated.h +++ b/tests/union_vector/union_vector_generated.h @@ -261,11 +261,11 @@ struct Movie FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && VerifyField(verifier, VT_MAIN_CHARACTER_TYPE) && - VerifyField(verifier, VT_MAIN_CHARACTER) && + VerifyOffset(verifier, VT_MAIN_CHARACTER) && VerifyCharacter(verifier, main_character(), main_character_type()) && - VerifyField(verifier, VT_CHARACTERS_TYPE) && + VerifyOffset(verifier, VT_CHARACTERS_TYPE) && verifier.Verify(characters_type()) && - VerifyField(verifier, VT_CHARACTERS) && + VerifyOffset(verifier, VT_CHARACTERS) && verifier.Verify(characters()) && VerifyCharacterVector(verifier, characters(), characters_type()) && verifier.EndTable();