Skip to content

Commit 23e507b

Browse files
author
Github Executorch
committed
Fix XNNPACK FlatBuffer verification and header bounds checking (TOB-EXECUTORCH-33, TOB-EXECUTORCH-34)
TOB-EXECUTORCH-33: XNNCompiler::compileModel() processed FlatBuffer data via fb_xnnpack::GetXNNGraph() without first running the FlatBuffer verifier. A malformed or truncated payload could cause out-of-bounds reads when the FlatBuffer library follows internal offset tables. This adds a flatbuffers::Verifier pass (matching the pattern used in program.cpp) before any FlatBuffer accessors are called, and tracks the flatbuffer_size so the verifier knows the exact bounds of the serialized data. TOB-EXECUTORCH-34: XNNHeader::Parse() read flatbuffer_offset, flatbuffer_size, constant_data_offset, and constant_data_size from untrusted header bytes but never validated that the resulting regions actually fit within the provided buffer. Crafted offset/size values could point past the end of the buffer, leading to out-of-bounds reads in compileModel(). This adds overflow-safe bounds checks that ensure both the flatbuffer and constant data regions fall within [0, size). This PR was authored with the assistance of Claude.
1 parent 21d9c64 commit 23e507b

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
@@ -1816,16 +1816,19 @@ ET_NODISCARD Error XNNCompiler::compileModel(
18161816
Result<XNNHeader> header = XNNHeader::Parse(buffer_pointer, num_bytes);
18171817
const uint8_t* flatbuffer_data = nullptr;
18181818
const uint8_t* constant_data = nullptr;
1819+
size_t flatbuffer_size = 0;
18191820
CompileAllocator compile_allocator;
18201821

18211822
// Header status can only either be Error::Ok or Error::NotFound
18221823
if (header.ok()) {
18231824
flatbuffer_data = reinterpret_cast<const uint8_t*>(buffer_pointer) +
18241825
header->flatbuffer_offset;
1826+
flatbuffer_size = header->flatbuffer_size;
18251827
constant_data = reinterpret_cast<const uint8_t*>(buffer_pointer) +
18261828
header->constant_data_offset;
18271829
} else if (header.error() == Error::NotFound) {
18281830
flatbuffer_data = reinterpret_cast<const uint8_t*>(buffer_pointer);
1831+
flatbuffer_size = num_bytes;
18291832
} else {
18301833
ET_LOG(Error, "XNNHeader may be corrupt");
18311834
return header.error();
@@ -1843,6 +1846,15 @@ ET_NODISCARD Error XNNCompiler::compileModel(
18431846
"XNNPACK Delegate Serialization Format version identifier '%.4s' != expected XN00 or XN01'",
18441847
flatbuffers::GetBufferIdentifier(flatbuffer_data));
18451848

1849+
// Verify the FlatBuffer data integrity before accessing it. Without this,
1850+
// malformed data could cause out-of-bounds reads when traversing the
1851+
// FlatBuffer's internal offset tables.
1852+
flatbuffers::Verifier verifier(flatbuffer_data, flatbuffer_size);
1853+
ET_CHECK_OR_RETURN_ERROR(
1854+
verifier.VerifyBuffer<fb_xnnpack::XNNGraph>(nullptr),
1855+
DelegateInvalidCompatibility,
1856+
"FlatBuffer verification failed; data may be truncated or corrupt");
1857+
18461858
auto flatbuffer_graph = fb_xnnpack::GetXNNGraph(flatbuffer_data);
18471859
// initialize xnnpack
18481860
xnn_status status = xnn_initialize(/*allocator =*/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)