Skip to content

Commit dbd5118

Browse files
lucylqGithub Executorch
andauthored
Fix XNNPACK FlatBuffer verification and header bounds checking (#18784)
1. Add flatbuffer verification to xnnpack graph 2. Check the flatbuffer and constant data region are valid (within flatbuffer size, and do not overlap with each other) This PR was authored with the assistance of Claude. Co-authored-by: Github Executorch <github_executorch@arm.com>
1 parent 7761476 commit dbd5118

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

backends/xnnpack/runtime/XNNCompiler.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1861,16 +1861,19 @@ ET_NODISCARD Error XNNCompiler::compileModel(
18611861
Result<XNNHeader> header = XNNHeader::Parse(buffer_pointer, num_bytes);
18621862
const uint8_t* flatbuffer_data = nullptr;
18631863
const uint8_t* constant_data = nullptr;
1864+
size_t flatbuffer_size = 0;
18641865
CompileAllocator compile_allocator;
18651866

18661867
// Header status can only either be Error::Ok or Error::NotFound
18671868
if (header.ok()) {
18681869
flatbuffer_data = reinterpret_cast<const uint8_t*>(buffer_pointer) +
18691870
header->flatbuffer_offset;
1871+
flatbuffer_size = header->flatbuffer_size;
18701872
constant_data = reinterpret_cast<const uint8_t*>(buffer_pointer) +
18711873
header->constant_data_offset;
18721874
} else if (header.error() == Error::NotFound) {
18731875
flatbuffer_data = reinterpret_cast<const uint8_t*>(buffer_pointer);
1876+
flatbuffer_size = num_bytes;
18741877
} else {
18751878
ET_LOG(Error, "XNNHeader may be corrupt");
18761879
return header.error();
@@ -1888,6 +1891,15 @@ ET_NODISCARD Error XNNCompiler::compileModel(
18881891
"XNNPACK Delegate Serialization Format version identifier '%.4s' != expected XN00 or XN01'",
18891892
flatbuffers::GetBufferIdentifier(flatbuffer_data));
18901893

1894+
// Verify the FlatBuffer data integrity before accessing it. Without this,
1895+
// malformed data could cause out-of-bounds reads when traversing the
1896+
// FlatBuffer's internal offset tables.
1897+
flatbuffers::Verifier verifier(flatbuffer_data, flatbuffer_size);
1898+
ET_CHECK_OR_RETURN_ERROR(
1899+
verifier.VerifyBuffer<fb_xnnpack::XNNGraph>(nullptr),
1900+
DelegateInvalidCompatibility,
1901+
"FlatBuffer verification failed; data may be truncated or corrupt");
1902+
18911903
auto flatbuffer_graph = fb_xnnpack::GetXNNGraph(flatbuffer_data);
18921904
ET_CHECK_OR_RETURN_ERROR(
18931905
flatbuffer_graph != nullptr && flatbuffer_graph->xvalues() != nullptr &&

backends/xnnpack/runtime/XNNHeader.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <executorch/backends/xnnpack/runtime/XNNHeader.h>
1010

11+
#include <cinttypes>
1112
#include <cstring>
1213

1314
#include <executorch/runtime/core/error.h>
@@ -64,6 +65,48 @@ Result<XNNHeader> XNNHeader::Parse(const void* data, size_t size) {
6465
uint64_t constant_data_size =
6566
GetUInt64LE(header_data + XNNHeader::kConstantDataSizeOffset);
6667

68+
// Validate min flatbuffer size.
69+
constexpr size_t kMinFlatbufferSize =
70+
sizeof(uint32_t) + 4; // root offset + identifier
71+
ET_CHECK_OR_RETURN_ERROR(
72+
flatbuffer_size >= kMinFlatbufferSize,
73+
InvalidArgument,
74+
"flatbuffer_size %" PRIu32 " is too small (minimum %zu)",
75+
flatbuffer_size,
76+
kMinFlatbufferSize);
77+
78+
// Validate that flatbuffer region does not overflow or exceed the buffer.
79+
ET_CHECK_OR_RETURN_ERROR(
80+
flatbuffer_offset <= size && flatbuffer_size <= size - flatbuffer_offset,
81+
InvalidArgument,
82+
"flatbuffer_offset: %" PRIu32 " and flatbuffer_size: %" PRIu32
83+
" are invalid for buffer of size: %zu",
84+
flatbuffer_offset,
85+
flatbuffer_size,
86+
size);
87+
// Validate that constant data region does not overflow or exceed the buffer.
88+
ET_CHECK_OR_RETURN_ERROR(
89+
constant_data_offset <= size &&
90+
constant_data_size <= size - constant_data_offset,
91+
InvalidArgument,
92+
"constant_data_offset: %" PRIu32 " and constant_data_size: %" PRIu64
93+
" are invalid for buffer of size: %zu",
94+
constant_data_offset,
95+
constant_data_size,
96+
size);
97+
98+
// Validate that constant data region does not overlap with flatbuffer region.
99+
// flatbuffer should come before constant data.
100+
ET_CHECK_OR_RETURN_ERROR(
101+
constant_data_offset >= flatbuffer_offset &&
102+
constant_data_offset - flatbuffer_offset >= flatbuffer_size,
103+
InvalidArgument,
104+
"constant_data_offset: %" PRIu32 " and flatbuffer_offset: %" PRIu32
105+
" with flatbuffer_size: %" PRIu32 " are overlapping.",
106+
constant_data_offset,
107+
flatbuffer_offset,
108+
flatbuffer_size);
109+
67110
return XNNHeader{
68111
flatbuffer_offset,
69112
flatbuffer_size,

0 commit comments

Comments
 (0)