Skip to content

Commit 6356067

Browse files
dbortfacebook-github-bot
authored andcommitted
Check for out-of-range op indices (#1516)
Summary: Ensure that op indices are in range before looking up values. Corrupted files with very large indices could cause arbitrary memory reads. Also ensure that the chains array is present before calling `size()` on it. Differential Revision: D52451740
1 parent 591c966 commit 6356067

File tree

1 file changed

+21
-4
lines changed

1 file changed

+21
-4
lines changed

runtime/executor/method.cpp

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,11 @@ Error Method::resolve_operator(
457457
constexpr size_t kTempBufferSizeForName = 100;
458458
char operator_name[kTempBufferSizeForName];
459459
const auto ops = serialization_plan_->operators();
460+
ET_CHECK_OR_RETURN_ERROR(
461+
ops != nullptr && op_index < ops->size(),
462+
InvalidProgram,
463+
"Op index %" PRIu32 " out of range",
464+
op_index);
460465
const auto& op = ops->Get(op_index);
461466

462467
populateOperatorName(op, kTempBufferSizeForName, operator_name);
@@ -567,11 +572,18 @@ Error Method::init(executorch_flatbuffer::ExecutionPlan* s_plan) {
567572
{
568573
// Load chains
569574
const auto chains = serialization_plan_->chains();
570-
ET_CHECK(chains != nullptr);
571-
n_chains_ = chains->size();
575+
if (chains == nullptr) {
576+
n_chains_ = 0;
577+
chains_ = nullptr;
578+
} else {
579+
n_chains_ = chains->size();
580+
chains_ =
581+
ET_ALLOCATE_LIST_OR_RETURN_ERROR(method_allocator, Chain, n_chains_);
582+
}
572583

573-
chains_ =
574-
ET_ALLOCATE_LIST_OR_RETURN_ERROR(method_allocator, Chain, n_chains_);
584+
// Try resolving all operators before failing, to make it easier to debug
585+
// multiple problems at once.
586+
Error delayed_error = Error::Ok;
575587
int32_t num_instructions_missing_op = 0;
576588
for (size_t i = 0; i < n_chains_; ++i) {
577589
auto s_chain = chains->Get(i);
@@ -610,6 +622,8 @@ Error Method::init(executorch_flatbuffer::ExecutionPlan* s_plan) {
610622
num_instructions_missing_op++;
611623
} else if (err == Error::MemoryAllocationFailed) {
612624
return err;
625+
} else {
626+
delayed_error = err;
613627
}
614628
} break;
615629
case executorch_flatbuffer::InstructionArguments::DelegateCall: {
@@ -644,6 +658,9 @@ Error Method::init(executorch_flatbuffer::ExecutionPlan* s_plan) {
644658
"There are %d instructions don't have corresponding operator registered. "
645659
"See logs for details",
646660
num_instructions_missing_op);
661+
if (delayed_error != Error::Ok) {
662+
return delayed_error;
663+
}
647664
}
648665

649666
pre_allocated_input_ = false;

0 commit comments

Comments
 (0)