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

diff --git a/docs/source/CppUsage.md b/docs/source/CppUsage.md
index 550c2c5..c269e1b 100755 (executable)
--- 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 (file)
--- 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<typename T> bool VerifyTable(const T *table) const {
+  template<typename T> 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<typename T> bool VerifyVectorOfTables(const Vector<Offset<T>> *vec)
-      const {
+  template<typename T> bool VerifyVectorOfTables(const Vector<Offset<T>> *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<typename T> bool VerifyBuffer() const {
+  template<typename T> bool VerifyBuffer() {
     // Call T::Verify, which must be in the generated code for this type.
     return Verify<uoffset_t>(buf_) &&
       reinterpret_cast<const T *>(buf_ + ReadScalar<uoffset_t>(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<soffset_t>(data_)) return false;
     auto vtable = data_ - ReadScalar<soffset_t>(data_);
     // Check the vtable size field, then check vtable fits in its entirety.
-    return verifier.Verify<voffset_t>(vtable) &&
+    return verifier.VerifyComplexity() &&
+           verifier.Verify<voffset_t>(vtable) &&
            verifier.Verify(vtable, ReadScalar<voffset_t>(vtable));
   }
 

diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp
index 46d6f09..b7fe8e1 100644 (file)
--- 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 (executable)
--- 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<uint8_t> *testnestedflatbuffer() const { return GetPointer<const flatbuffers::Vector<uint8_t> *>(30); }
   const Monster *testnestedflatbuffer_nested_root() { return flatbuffers::GetRoot<Monster>(testnestedflatbuffer()->Data()); }
   const Monster *testempty() const { return GetPointer<const Monster *>(32); }
-  bool Verify(const flatbuffers::Verifier &verifier) const {
-    return VerifyTable(verifier) &&
+  bool Verify(flatbuffers::Verifier &verifier) const {
+    return VerifyTableStart(verifier) &&
            VerifyField<Vec3>(verifier, 4 /* pos */) &&
            VerifyField<int16_t>(verifier, 6 /* mana */) &&
            VerifyField<int16_t>(verifier, 8 /* hp */) &&
@@ -128,7 +128,8 @@ struct Monster : private flatbuffers::Table {
            VerifyField<flatbuffers::uoffset_t>(verifier, 30 /* testnestedflatbuffer */) &&
            verifier.Verify(testnestedflatbuffer()) &&
            VerifyField<flatbuffers::uoffset_t>(verifier, 32 /* testempty */) &&
-           verifier.VerifyTable(testempty());
+           verifier.VerifyTable(testempty()) &&
+           verifier.EndTable();
   }
 };
 
@@ -187,7 +188,7 @@ inline flatbuffers::Offset<Monster> 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<const Monster *>(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<Monster>(buf); }
 
-inline bool VerifyMonsterBuffer(const flatbuffers::Verifier &verifier) { return verifier.VerifyBuffer<Monster>(); }
+inline bool VerifyMonsterBuffer(flatbuffers::Verifier &verifier) { return verifier.VerifyBuffer<Monster>(); }
 
 inline void FinishMonsterBuffer(flatbuffers::FlatBufferBuilder &fbb, flatbuffers::Offset<Monster> root) { fbb.Finish(root, "MONS"); }