Skip to content

Commit 91e372b

Browse files
author
Github Executorch
committed
Fix TOB-EXECUTORCH-41: validate buffer_idx bounds in getConstantDataPtr
Add bounds checking on buffer_idx in both constant_buffer and constant_data code paths of getConstantDataPtr to prevent out-of-bounds vector access from malicious flatbuffer inputs. Authored-with: Claude
1 parent 5e8a0df commit 91e372b

1 file changed

Lines changed: 57 additions & 16 deletions

File tree

backends/xnnpack/runtime/XNNCompiler.cpp

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,12 @@ std::vector<T> flatbufferDimsToVector(
170170
/**
171171
Gets the constant data pointer associated with the given tensor value.
172172
Obtaining the constant data pointer can either be from within the flatbuffer
173-
payload (deprecated) or via offsets to the constant_data_ptr. If no constant
174-
data associated with the tensor value, then returns nullptr.
173+
payload (deprecated) or via offsets to the constant_data_ptr.
174+
175+
Failures are returned as an Error, and the successful value may be nullptr
176+
when the tensor has no associated constant data.
175177
*/
176-
const uint8_t* getConstantDataPtr(
178+
Result<const uint8_t*> getConstantDataPtr(
177179
uint32_t buffer_idx,
178180
GraphPtr flatbuffer_graph,
179181
const uint8_t* constant_data_ptr,
@@ -184,13 +186,39 @@ const uint8_t* getConstantDataPtr(
184186
if (!constant_data_ptr) {
185187
// TODO(T172265611): Remove constant_buffer in flatbuffer path after BC
186188
// window
187-
const auto& constant_buffer = *flatbuffer_graph->constant_buffer();
188-
return constant_buffer[buffer_idx]->storage()->data();
189+
auto* cb = flatbuffer_graph->constant_buffer();
190+
ET_CHECK_OR_RETURN_ERROR(
191+
cb != nullptr, InvalidProgram, "constant_buffer is null");
192+
ET_CHECK_OR_RETURN_ERROR(
193+
buffer_idx < cb->size(),
194+
InvalidProgram,
195+
"buffer_idx %u out of bounds for constant_buffer of size %zu",
196+
buffer_idx,
197+
cb->size());
198+
auto* buffer_entry = (*cb)[buffer_idx];
199+
ET_CHECK_OR_RETURN_ERROR(
200+
buffer_entry != nullptr && buffer_entry->storage() != nullptr,
201+
InvalidProgram,
202+
"Null constant_buffer entry at buffer_idx %u",
203+
buffer_idx);
204+
return buffer_entry->storage()->data();
189205
} else {
190-
ConstantDataOffsetPtr constant_data_offset =
191-
flatbuffer_graph->constant_data()->Get(buffer_idx);
206+
auto* cd = flatbuffer_graph->constant_data();
207+
ET_CHECK_OR_RETURN_ERROR(
208+
cd != nullptr, InvalidProgram, "constant_data is null");
209+
ET_CHECK_OR_RETURN_ERROR(
210+
buffer_idx < cd->size(),
211+
InvalidProgram,
212+
"buffer_idx %u out of bounds for constant_data of size %zu",
213+
buffer_idx,
214+
cd->size());
215+
ConstantDataOffsetPtr constant_data_offset = cd->Get(buffer_idx);
216+
ET_CHECK_OR_RETURN_ERROR(
217+
constant_data_offset != nullptr,
218+
InvalidProgram,
219+
"Null constant_data entry at buffer_idx %u",
220+
buffer_idx);
192221
uint64_t offset = constant_data_offset->offset();
193-
194222
bool has_named_key = flatbuffers::IsFieldPresent(
195223
constant_data_offset, fb_xnnpack::ConstantDataOffset::VT_NAMED_KEY);
196224
// If there is no tensor name
@@ -203,7 +231,7 @@ const uint8_t* getConstantDataPtr(
203231
weights_cache->load_unpacked_data(data_name);
204232
if (!data_ptr.ok()) {
205233
ET_LOG(Error, "Failed to load weights from cache");
206-
return nullptr;
234+
return data_ptr.error();
207235
}
208236
return data_ptr.get();
209237
#else
@@ -215,7 +243,7 @@ const uint8_t* getConstantDataPtr(
215243
"Failed to get constant data for key %s from named_data_map. Error code: %u",
216244
data_name.c_str(),
217245
static_cast<uint32_t>(buffer.error()));
218-
return nullptr;
246+
return buffer.error();
219247
}
220248
const uint8_t* data_ptr =
221249
static_cast<const uint8_t*>(buffer.get().data());
@@ -229,7 +257,7 @@ const uint8_t* getConstantDataPtr(
229257
return nullptr;
230258
}
231259

232-
const uint8_t* getConstantDataPtr(
260+
Result<const uint8_t*> getConstantDataPtr(
233261
const fb_xnnpack::XNNTensorValue* tensor_value,
234262
GraphPtr flatbuffer_graph,
235263
const uint8_t* constant_data_ptr,
@@ -298,13 +326,17 @@ Error defineTensor(
298326

299327
// Get Pointer to constant data from flatbuffer, if its non-constant
300328
// it is a nullptr
301-
const uint8_t* buffer_ptr = getConstantDataPtr(
329+
auto buffer_result = getConstantDataPtr(
302330
tensor_value,
303331
flatbuffer_graph,
304332
constant_data_ptr,
305333
named_data_map,
306334
freeable_buffers,
307335
weights_cache);
336+
if (!buffer_result.ok()) {
337+
return buffer_result.error();
338+
}
339+
const uint8_t* buffer_ptr = buffer_result.get();
308340

309341
xnn_status status;
310342
// The type we might have to convert to
@@ -449,13 +481,17 @@ Error defineTensor(
449481
const float* scale = qparams->scale()->data();
450482

451483
if (qparams->scale_buffer_idx() != 0) {
452-
scale = reinterpret_cast<const float*>(getConstantDataPtr(
484+
auto scale_result = getConstantDataPtr(
453485
qparams->scale_buffer_idx(),
454486
flatbuffer_graph,
455487
constant_data_ptr,
456488
named_data_map,
457489
freeable_buffers,
458-
weights_cache));
490+
weights_cache);
491+
if (!scale_result.ok()) {
492+
return scale_result.error();
493+
}
494+
scale = reinterpret_cast<const float*>(scale_result.get());
459495
ET_CHECK_OR_RETURN_ERROR(
460496
scale != nullptr, Internal, "Failed to load scale data.");
461497
}
@@ -491,13 +527,18 @@ Error defineTensor(
491527
// Block scales are preferably serialized as bf16 but can also be
492528
// serialized as fp32 for backwards compatability.
493529
if (qparams->scale_buffer_idx() != 0) {
494-
scale_data = reinterpret_cast<const uint16_t*>(getConstantDataPtr(
530+
auto scale_data_result = getConstantDataPtr(
495531
qparams->scale_buffer_idx(),
496532
flatbuffer_graph,
497533
constant_data_ptr,
498534
named_data_map,
499535
freeable_buffers,
500-
weights_cache));
536+
weights_cache);
537+
if (!scale_data_result.ok()) {
538+
return scale_data_result.error();
539+
}
540+
scale_data =
541+
reinterpret_cast<const uint16_t*>(scale_data_result.get());
501542
ET_CHECK_OR_RETURN_ERROR(
502543
scale_data != nullptr, Internal, "Failed to load scale data.");
503544
scale_numel = qparams->num_scales();

0 commit comments

Comments
 (0)