From d6722e7b9cdbc7bbb2e41bbd247776953bc252a3 Mon Sep 17 00:00:00 2001 From: Derek Bailey Date: Thu, 11 May 2023 11:27:26 -0700 Subject: [PATCH] Change SizedPrefixed verifier to be <= provided size --- include/flatbuffers/verifier.h | 16 +++++++++------- tests/monster_test.cpp | 24 +++++++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/include/flatbuffers/verifier.h b/include/flatbuffers/verifier.h index c64737e4962..167059f7a63 100644 --- a/include/flatbuffers/verifier.h +++ b/include/flatbuffers/verifier.h @@ -60,7 +60,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { // Central location where any verification failures register. bool Check(const bool ok) const { - // clang-format off +// clang-format off #ifdef FLATBUFFERS_DEBUG_VERIFICATION_FAILURE if (opts_.assert) { FLATBUFFERS_ASSERT(ok); } #endif @@ -74,7 +74,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { // Verify any range within the buffer. bool Verify(const size_t elem, const size_t elem_len) const { - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE auto upper_bound = elem + elem_len; if (upper_bound_ < upper_bound) @@ -177,8 +177,8 @@ class Verifier FLATBUFFERS_FINAL_CLASS { return true; } - FLATBUFFERS_SUPPRESS_UBSAN("unsigned-integer-overflow") bool VerifyTableStart( - const uint8_t *const table) { + FLATBUFFERS_SUPPRESS_UBSAN("unsigned-integer-overflow") + bool VerifyTableStart(const uint8_t *const table) { // Check the vtable offset. const auto tableo = static_cast(table - buf_); if (!Verify(tableo)) return false; @@ -212,7 +212,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { const auto o = VerifyOffset(start); return Check(o != 0) && reinterpret_cast(buf_ + start + o)->Verify(*this) - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE && GetComputedSize() #endif @@ -246,7 +246,9 @@ class Verifier FLATBUFFERS_FINAL_CLASS { template bool VerifySizePrefixedBuffer(const char *const identifier) { return Verify(0U) && - Check(ReadScalar(buf_) == size_ - sizeof(SizeT)) && + // Ensure the prefixed size is within the bounds of the provided + // length. + Check(ReadScalar(buf_) + sizeof(SizeT) <= size_) && VerifyBufferFromStart(identifier, sizeof(SizeT)); } @@ -286,7 +288,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { // Returns the message size in bytes size_t GetComputedSize() const { - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE uintptr_t size = upper_bound_; // Align the size to uoffset_t diff --git a/tests/monster_test.cpp b/tests/monster_test.cpp index 2ca3277a9be..bae47490321 100644 --- a/tests/monster_test.cpp +++ b/tests/monster_test.cpp @@ -171,7 +171,7 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { FinishMonsterBuffer(builder, mloc); - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TEST_VERBOSE // print byte data for debugging: auto p = builder.GetBufferPointer(); @@ -196,7 +196,7 @@ void AccessFlatBufferTest(const uint8_t *flatbuf, size_t length, bool pooled) { verifier.SetFlexReuseTracker(&flex_reuse_tracker); TEST_EQ(VerifyMonsterBuffer(verifier), true); - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE std::vector test_buff; test_buff.resize(length * 2); @@ -422,8 +422,8 @@ void MutateFlatBuffersTest(uint8_t *flatbuf, std::size_t length) { // Mutate structs. auto pos = monster->mutable_pos(); - auto & test3 = pos->mutable_test3(); // Struct inside a struct. - test3.mutate_a(50); // Struct fields never fail. + auto &test3 = pos->mutable_test3(); // Struct inside a struct. + test3.mutate_a(50); // Struct fields never fail. TEST_EQ(test3.a(), 50); test3.mutate_a(10); @@ -441,13 +441,12 @@ void MutateFlatBuffersTest(uint8_t *flatbuf, std::size_t length) { first->mutate_hp(1000); // Test for each loop over mutable entries - for (auto item: *tables) - { + for (auto item : *tables) { TEST_EQ(item->hp(), 1000); item->mutate_hp(0); TEST_EQ(item->hp(), 0); item->mutate_hp(1000); - break; // one iteration is enough, just testing compilation + break; // one iteration is enough, just testing compilation } // Mutate via LookupByKey @@ -584,6 +583,17 @@ void SizePrefixedTest() { TEST_EQ(m->mana(), 200); TEST_EQ(m->hp(), 300); TEST_EQ_STR(m->name()->c_str(), "bob"); + + { + // Verify that passing a larger size is OK, but not a smaller + flatbuffers::Verifier verifier_larger(fbb.GetBufferPointer(), + fbb.GetSize() + 10); + TEST_EQ(VerifySizePrefixedMonsterBuffer(verifier_larger), true); + + flatbuffers::Verifier verifier_smaller(fbb.GetBufferPointer(), + fbb.GetSize() - 10); + TEST_EQ(VerifySizePrefixedMonsterBuffer(verifier_smaller), false); + } } void TestMonsterExtraFloats(const std::string &tests_data_path) {