Skip to content

Commit

Permalink
ARROW-11559: [C++] Use smarter Flatbuffers verification parameters
Browse files Browse the repository at this point in the history
Flatbuffers is able to encode a virtually unbounded of schema fields in a small buffer size.
Verifying that many fields with the Flatbuffers verifier seems to result in potentially
unbounded verification times, which is a denial of service risk.

To mitigate the risk, impose that a Flatbuffers buffer cannot represent one more than
one Flatbuffers table per buffer bit, which should always be true for well-formed
Arrow IPC metadata.  Indeed, the only recursive table, the `Field` table in Schema.fbs,
mandates the presence of its `type` member (though it's not marked as required in
the Flatbuffers definition, it's validated by the IPC read routines).
  • Loading branch information
pitrou committed Feb 8, 2021
1 parent 1c219e3 commit bad3f4c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
15 changes: 13 additions & 2 deletions cpp/src/arrow/ipc/metadata_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,21 @@ Status GetSparseTensorMetadata(const Buffer& metadata, std::shared_ptr<DataType>
Status GetKeyValueMetadata(const KVVector* fb_metadata,
std::shared_ptr<KeyValueMetadata>* out);

template <typename RootType>
bool VerifyFlatbuffers(const uint8_t* data, int64_t size) {
// Heuristic: tables in a Arrow flatbuffers buffer must take at least 1 bit
// each in average (ARROW-11559).
// Especially, the only recursive table (the `Field` table in Schema.fbs)
// must have a non-empty `type` member.
flatbuffers::Verifier verifier(data, static_cast<flatbuffers::uoffset_t>(size),
/*max_depth=*/128,
/*max_tables=*/8 * size);
return verifier.VerifyBuffer<RootType>(nullptr);
}

static inline Status VerifyMessage(const uint8_t* data, int64_t size,
const flatbuf::Message** out) {
flatbuffers::Verifier verifier(data, size, /*max_depth=*/128);
if (!flatbuf::VerifyMessageBuffer(verifier)) {
if (!VerifyFlatbuffers<flatbuf::Message>(data, size)) {
return Status::IOError("Invalid flatbuffers message.");
}
*out = flatbuf::GetMessage(data);
Expand Down
8 changes: 3 additions & 5 deletions cpp/src/arrow/ipc/reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1052,11 +1052,9 @@ class RecordBatchFileReaderImpl : public RecordBatchFileReader {
footer_buffer_,
file_->ReadAt(footer_offset_ - footer_length - file_end_size, footer_length));

auto data = footer_buffer_->data();
flatbuffers::Verifier verifier(data, footer_buffer_->size(), /*max_depth=*/128,
/*max_tables=*/UINT_MAX);

if (!flatbuf::VerifyFooterBuffer(verifier)) {
const auto data = footer_buffer_->data();
const auto size = footer_buffer_->size();
if (!internal::VerifyFlatbuffers<flatbuf::Footer>(data, size)) {
return Status::IOError("Verification of flatbuffer-encoded Footer failed.");
}
footer_ = flatbuf::GetFooter(data);
Expand Down

0 comments on commit bad3f4c

Please sign in to comment.