Skip to content

Commit dca5ef1

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 dca5ef1

2 files changed

Lines changed: 43 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+
fb_xnnpack::VerifyXNNGraphBuffer(verifier),
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: 31 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,36 @@ 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 that flatbuffer region does not overflow or exceed the buffer.
69+
ET_CHECK_OR_RETURN_ERROR(
70+
flatbuffer_offset <= size && flatbuffer_size <= size - flatbuffer_offset,
71+
InvalidArgument,
72+
"flatbuffer_offset: %u and flatbuffer_size: %u are invalid for buffer of size: %zu",
73+
flatbuffer_offset,
74+
flatbuffer_size,
75+
size);
76+
// Validate that constant data region does not overflow or exceed the buffer.
77+
ET_CHECK_OR_RETURN_ERROR(
78+
constant_data_offset <= size &&
79+
constant_data_size <= size - constant_data_offset,
80+
InvalidArgument,
81+
"constant_data_offset: %u and constant_data_size: %" PRIu64
82+
" are invalid for buffer of size: %zu",
83+
constant_data_offset,
84+
constant_data_size,
85+
size);
86+
87+
// Validate that constant data region does not overlap with flatbuffer region.
88+
// flatbuffer should come before constant data.
89+
ET_CHECK_OR_RETURN_ERROR(
90+
constant_data_offset >= flatbuffer_offset &&
91+
constant_data_offset - flatbuffer_offset >= flatbuffer_size,
92+
InvalidArgument,
93+
"constant_data_offset: %u and flatbuffer_offset: %u with flatbuffer_size: %u are overlapping.",
94+
constant_data_offset,
95+
flatbuffer_offset,
96+
flatbuffer_size);
97+
6798
return XNNHeader{
6899
flatbuffer_offset,
69100
flatbuffer_size,

0 commit comments

Comments
 (0)