Skip to content

Commit

Permalink
Change SizedPrefixed verifier to be <= provided size (google#7957)
Browse files Browse the repository at this point in the history
* Change SizedPrefixed verifier to be <= provided size

* add GetSizePrefixedBufferLength()
  • Loading branch information
dbaileychess authored and Jochen Parmentier committed Oct 29, 2024
1 parent 91dc91f commit 3b33235
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 8 deletions.
12 changes: 12 additions & 0 deletions include/flatbuffers/flatbuffers.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,18 @@ inline SizeT GetPrefixedSize(const uint8_t *buf) {
return ReadScalar<SizeT>(buf);
}

// Gets the total length of the buffer given a sized prefixed FlatBuffer.
//
// This includes the size of the prefix as well as the buffer:
//
// [size prefix][flatbuffer]
// |---------length--------|
template<typename SizeT = uoffset_t>
inline SizeT GetSizePrefixedBufferLength(const uint8_t * const buf) {
return ReadScalar<SizeT>(buf) + sizeof(SizeT);
}


// Base class for native objects (FlatBuffer data de-serialized into native
// C++ data structures).
// Contains no functionality, purely documentative.
Expand Down
8 changes: 5 additions & 3 deletions include/flatbuffers/verifier.h
Original file line number Diff line number Diff line change
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 @@ -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
31 changes: 26 additions & 5 deletions tests/monster_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
#include <limits>
#include <vector>

#include "flatbuffers/base.h"
#include "flatbuffers/flatbuffer_builder.h"
#include "flatbuffers/flatbuffers.h"
#include "flatbuffers/idl.h"
#include "flatbuffers/registry.h"
#include "flatbuffers/verifier.h"
Expand Down Expand Up @@ -422,8 +424,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 +443,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 @@ -579,11 +580,31 @@ void SizePrefixedTest() {
flatbuffers::Verifier verifier(fbb.GetBufferPointer(), fbb.GetSize());
TEST_EQ(VerifySizePrefixedMonsterBuffer(verifier), true);

// The prefixed size doesn't include itself, so substract the size of the
// prefix
TEST_EQ(GetPrefixedSize(fbb.GetBufferPointer()),
fbb.GetSize() - sizeof(uoffset_t));

// Getting the buffer length does include the prefix size, so it should be the
// full lenght.
TEST_EQ(GetSizePrefixedBufferLength(fbb.GetBufferPointer()), fbb.GetSize());

// Access it.
auto m = GetSizePrefixedMonster(fbb.GetBufferPointer());
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 3b33235

Please sign in to comment.