Skip to content

Commit

Permalink
Change SizedPrefixed verifier to be <= provided size
Browse files Browse the repository at this point in the history
  • Loading branch information
dbaileychess committed May 11, 2023
1 parent 66e9d98 commit d6722e7
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
16 changes: 9 additions & 7 deletions include/flatbuffers/verifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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<size_t>(table - buf_);
if (!Verify<soffset_t>(tableo)) return false;
Expand Down Expand Up @@ -212,7 +212,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS {
const auto o = VerifyOffset<uoffset_t>(start);
return Check(o != 0) &&
reinterpret_cast<const T *>(buf_ + start + o)->Verify(*this)
// clang-format off
// clang-format off
#ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE
&& GetComputedSize()
#endif
Expand Down Expand Up @@ -246,7 +246,9 @@ class Verifier FLATBUFFERS_FINAL_CLASS {
template<typename T, typename SizeT = uoffset_t>
bool VerifySizePrefixedBuffer(const char *const identifier) {
return Verify<SizeT>(0U) &&
Check(ReadScalar<SizeT>(buf_) == size_ - sizeof(SizeT)) &&
// Ensure the prefixed size is within the bounds of the provided
// length.
Check(ReadScalar<SizeT>(buf_) + sizeof(SizeT) <= size_) &&
VerifyBufferFromStart<T>(identifier, sizeof(SizeT));
}

Expand Down Expand Up @@ -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
Expand Down
24 changes: 17 additions & 7 deletions tests/monster_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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<uint8_t> test_buff;
test_buff.resize(length * 2);
Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit d6722e7

Please sign in to comment.