From 11b743688c0fedd4cb561ea0bf4a1d9d81d5663c Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Thu, 21 Aug 2014 17:00:54 -0700 Subject: [PATCH] Improved the verifier to be even more resilient. Theoretically, an attacker could construct a FlatBuffer with the sole purpose of making verification really expensive, essentially DOS-ing a server that uses verification on FlatBuffers. This adds a max table depth and max table amount at which point the verifier declares the buffer malformed. Bug: 16301336 Change-Id: I6b098c31d030d24c19e852b33609110658e66aa9 Tested: on OS X --- docs/source/CppUsage.md | 5 +++++ include/flatbuffers/flatbuffers.h | 42 +++++++++++++++++++++++++++++++-------- src/idl_gen_cpp.cpp | 9 +++++---- tests/monster_test_generated.h | 13 ++++++------ 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/docs/source/CppUsage.md b/docs/source/CppUsage.md index 550c2c5..c269e1b 100755 --- a/docs/source/CppUsage.md +++ b/docs/source/CppUsage.md @@ -188,6 +188,11 @@ a full traversal (since any scalar data is not actually touched), and since it may cause the buffer to be brought into cache before reading, the actual overhead may be even lower than expected. +In specialized cases where a denial of service attack is possible, +the verifier has two additional constructor arguments that allow +you to limit the nesting depth and total amount of tables the +verifier may encounter before declaring the buffer malformed. + ## Text & schema parsing Using binary buffers with the generated header provides a super low diff --git a/include/flatbuffers/flatbuffers.h b/include/flatbuffers/flatbuffers.h index 91dce85..96482e4 100644 --- a/include/flatbuffers/flatbuffers.h +++ b/include/flatbuffers/flatbuffers.h @@ -669,8 +669,10 @@ inline bool BufferHasIdentifier(const void *buf, const char *identifier) { // Helper class to verify the integrity of a FlatBuffer class Verifier { public: - Verifier(const uint8_t *buf, size_t buf_len) - : buf_(buf), end_(buf + buf_len) + Verifier(const uint8_t *buf, size_t buf_len, size_t _max_depth = 64, + size_t _max_tables = 1000000) + : buf_(buf), end_(buf + buf_len), depth_(0), max_depth_(_max_depth), + num_tables_(0), max_tables_(_max_tables) {} // Verify any range within the buffer. @@ -688,7 +690,7 @@ class Verifier { } // Verify a pointer (may be NULL) of a table type. - template bool VerifyTable(const T *table) const { + template bool VerifyTable(const T *table) { return !table || table->Verify(*this); } @@ -733,8 +735,7 @@ class Verifier { } // Special case for table contents, after the above has been called. - template bool VerifyVectorOfTables(const Vector> *vec) - const { + template bool VerifyVectorOfTables(const Vector> *vec) { if (vec) { for (uoffset_t i = 0; i < vec->Length(); i++) { if (!vec->Get(i)->Verify(*this)) return false; @@ -744,16 +745,40 @@ class Verifier { } // Verify this whole buffer, starting with root type T. - template bool VerifyBuffer() const { + template bool VerifyBuffer() { // Call T::Verify, which must be in the generated code for this type. return Verify(buf_) && reinterpret_cast(buf_ + ReadScalar(buf_))-> Verify(*this); } + // 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 + // with EndTable(). + bool VerifyComplexity() { + depth_++; + num_tables_++; + bool too_complex = depth_ > max_depth_ || num_tables_ > max_tables_; + #ifdef FLATBUFFERS_DEBUG_VERIFICATION_FAILURE + assert(!too_complex); + #endif + return !too_complex; + } + + // Called at the end of a table to pop the depth count. + bool EndTable() { + depth_--; + return true; + } + private: const uint8_t *buf_; const uint8_t *end_; + size_t depth_; + size_t max_depth_; + size_t num_tables_; + size_t max_tables_; }; // "structs" are flat structures that do not have an offset table, thus @@ -828,12 +853,13 @@ class Table { // Verify the vtable of this table. // Call this once per table, followed by VerifyField once per field. - bool VerifyTable(const Verifier &verifier) const { + bool VerifyTableStart(Verifier &verifier) const { // Check the vtable offset. if (!verifier.Verify(data_)) return false; auto vtable = data_ - ReadScalar(data_); // Check the vtable size field, then check vtable fits in its entirety. - return verifier.Verify(vtable) && + return verifier.VerifyComplexity() && + verifier.Verify(vtable) && verifier.Verify(vtable, ReadScalar(vtable)); } diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index 46d6f09..b7fe8e1 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -166,7 +166,7 @@ static void GenEnum(EnumDef &enum_def, std::string *code_ptr, // has been corrupted, since the verifiers will simply fail when called // on the wrong type. auto signature = "bool Verify" + enum_def.name + - "(const flatbuffers::Verifier &verifier, " + + "(flatbuffers::Verifier &verifier, " + "const void *union_obj, uint8_t type)"; code += signature + ";\n\n"; code_post += signature + " {\n switch (type) {\n"; @@ -227,8 +227,8 @@ static void GenTable(const Parser &parser, StructDef &struct_def, } // Generate a verifier function that can check a buffer from an untrusted // source will never cause reads outside the buffer. - code += " bool Verify(const flatbuffers::Verifier &verifier) const {\n"; - code += " return VerifyTable(verifier)"; + code += " bool Verify(flatbuffers::Verifier &verifier) const {\n"; + code += " return VerifyTableStart(verifier)"; std::string prefix = " &&\n "; for (auto it = struct_def.fields.vec.begin(); it != struct_def.fields.vec.end(); @@ -276,6 +276,7 @@ static void GenTable(const Parser &parser, StructDef &struct_def, } } } + code += prefix + "verifier.EndTable()"; code += ";\n }\n"; code += "};\n\n"; @@ -551,7 +552,7 @@ std::string GenerateCPP(const Parser &parser, // The root verifier: code += "inline bool Verify"; code += parser.root_struct_def->name; - code += "Buffer(const flatbuffers::Verifier &verifier) { " + code += "Buffer(flatbuffers::Verifier &verifier) { " "return verifier.VerifyBuffer<"; code += parser.root_struct_def->name + ">(); }\n\n"; diff --git a/tests/monster_test_generated.h b/tests/monster_test_generated.h index bf0719e..8c45a40 100755 --- a/tests/monster_test_generated.h +++ b/tests/monster_test_generated.h @@ -43,7 +43,7 @@ inline const char **EnumNamesAny() { inline const char *EnumNameAny(int e) { return EnumNamesAny()[e]; } -bool VerifyAny(const flatbuffers::Verifier &verifier, const void *union_obj, uint8_t type); +bool VerifyAny(flatbuffers::Verifier &verifier, const void *union_obj, uint8_t type); MANUALLY_ALIGNED_STRUCT(2) Test { private: @@ -102,8 +102,8 @@ struct Monster : private flatbuffers::Table { const flatbuffers::Vector *testnestedflatbuffer() const { return GetPointer *>(30); } const Monster *testnestedflatbuffer_nested_root() { return flatbuffers::GetRoot(testnestedflatbuffer()->Data()); } const Monster *testempty() const { return GetPointer(32); } - bool Verify(const flatbuffers::Verifier &verifier) const { - return VerifyTable(verifier) && + bool Verify(flatbuffers::Verifier &verifier) const { + return VerifyTableStart(verifier) && VerifyField(verifier, 4 /* pos */) && VerifyField(verifier, 6 /* mana */) && VerifyField(verifier, 8 /* hp */) && @@ -128,7 +128,8 @@ struct Monster : private flatbuffers::Table { VerifyField(verifier, 30 /* testnestedflatbuffer */) && verifier.Verify(testnestedflatbuffer()) && VerifyField(verifier, 32 /* testempty */) && - verifier.VerifyTable(testempty()); + verifier.VerifyTable(testempty()) && + verifier.EndTable(); } }; @@ -187,7 +188,7 @@ inline flatbuffers::Offset CreateMonster(flatbuffers::FlatBufferBuilder return builder_.Finish(); } -bool VerifyAny(const flatbuffers::Verifier &verifier, const void *union_obj, uint8_t type) { +bool VerifyAny(flatbuffers::Verifier &verifier, const void *union_obj, uint8_t type) { switch (type) { case Any_NONE: return true; case Any_Monster: return verifier.VerifyTable(reinterpret_cast(union_obj)); @@ -197,7 +198,7 @@ bool VerifyAny(const flatbuffers::Verifier &verifier, const void *union_obj, uin inline const Monster *GetMonster(const void *buf) { return flatbuffers::GetRoot(buf); } -inline bool VerifyMonsterBuffer(const flatbuffers::Verifier &verifier) { return verifier.VerifyBuffer(); } +inline bool VerifyMonsterBuffer(flatbuffers::Verifier &verifier) { return verifier.VerifyBuffer(); } inline void FinishMonsterBuffer(flatbuffers::FlatBufferBuilder &fbb, flatbuffers::Offset root) { fbb.Finish(root, "MONS"); } -- 2.7.4