Skip to content

Commit 3c586d6

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 3c586d6

2 files changed

Lines changed: 43 additions & 1 deletion

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 & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88

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

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

1314
#include <executorch/runtime/core/error.h>
1415
#include <executorch/runtime/core/result.h>
15-
1616
#pragma clang diagnostic ignored "-Wdeprecated"
1717

1818
namespace executorch {
@@ -64,6 +64,36 @@ Result<XNNHeader> XNNHeader::Parse(const void* data, size_t size) {
6464
uint64_t constant_data_size =
6565
GetUInt64LE(header_data + XNNHeader::kConstantDataSizeOffset);
6666

67+
// Validate that flatbuffer region does not overflow or exceed the buffer.
68+
ET_CHECK_OR_RETURN_ERROR(
69+
flatbuffer_offset <= size && flatbuffer_size <= size - flatbuffer_offset,
70+
InvalidArgument,
71+
"flatbuffer_offset: %u and flatbuffer_size: %u are invalid for buffer of size: %zu",
72+
flatbuffer_offset,
73+
flatbuffer_size,
74+
size);
75+
// Validate that constant data region does not overflow or exceed the buffer.
76+
ET_CHECK_OR_RETURN_ERROR(
77+
constant_data_offset <= size &&
78+
constant_data_size <= size - constant_data_offset,
79+
InvalidArgument,
80+
"constant_data_offset: %u and constant_data_size: %" PRIu64
81+
" are invalid for buffer of size: %zu",
82+
constant_data_offset,
83+
constant_data_size,
84+
size);
85+
86+
// Validate that constant data region does not overlap with flatbuffer region.
87+
// flatbuffer should come before constant data.
88+
ET_CHECK_OR_RETURN_ERROR(
89+
constant_data_offset >= flatbuffer_offset &&
90+
constant_data_offset - flatbuffer_offset >= flatbuffer_size,
91+
InvalidArgument,
92+
"constant_data_offset: %u and flatbuffer_offset: %u with flatbuffer_size: %u are overlapping.",
93+
constant_data_offset,
94+
flatbuffer_offset,
95+
flatbuffer_size);
96+
6797
return XNNHeader{
6898
flatbuffer_offset,
6999
flatbuffer_size,

0 commit comments

Comments
 (0)