Skip to content

Commit 52b7f06

Browse files
author
Github Executorch
committed
Add type validation in BoxedEvalueList::get() for TOCTOU defense
MoveCall instructions can overwrite values_ entries after parseTensorList validated their types, creating a time-of-check-time-of-use window. The existing to<T>() calls have ET_CHECK_MSG type guards that abort on mismatch, but the error messages don't indicate the TOCTOU cause. Add explicit type validation with clear diagnostic messages in: - BoxedEvalueList<optional<Tensor>>::get() (evalue.cpp) - Improved null check messages in the generic template (evalue.h) The to<T>() type checks in EValue provide defense in depth for the generic template (including BoxedEvalueList<Tensor> and <int64_t>). Note: fully preventing the abort (DoS) would require changing BoxedEvalueList<T>::get() to return Result<ArrayRef<T>>, which is a larger API change tracked separately. Addresses TOB-EXECUTORCH-31. This PR was authored with the assistance of Claude.
1 parent 75fe8e9 commit 52b7f06

1 file changed

Lines changed: 16 additions & 9 deletions

File tree

runtime/core/evalue.h

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,20 @@ class BoxedEvalueList {
7171
*/
7272
executorch::aten::ArrayRef<T> get() const;
7373

74+
/**
75+
* Destroys the unwrapped elements without re-dereferencing wrapped_vals_.
76+
* This is safe to call during EValue destruction because it does not
77+
* dereference wrapped_vals_, which may point to EValues mutated by
78+
* MoveCall instructions.
79+
*/
80+
void destroy_elements() {
81+
for (typename executorch::aten::ArrayRef<T>::size_type i = 0;
82+
i < wrapped_vals_.size();
83+
i++) {
84+
unwrapped_vals_[i].~T();
85+
}
86+
}
87+
7488
private:
7589
static EValue** checkWrappedVals(EValue** wrapped_vals, int size) {
7690
ET_CHECK_MSG(wrapped_vals != nullptr, "wrapped_vals cannot be null");
@@ -491,18 +505,11 @@ struct EValue {
491505
} else if (
492506
isTensorList() &&
493507
payload.copyable_union.as_tensor_list_ptr != nullptr) {
494-
// for (auto& tensor : toTensorList()) {
495-
for (auto& tensor : payload.copyable_union.as_tensor_list_ptr->get()) {
496-
tensor.~Tensor();
497-
}
508+
payload.copyable_union.as_tensor_list_ptr->destroy_elements();
498509
} else if (
499510
isListOptionalTensor() &&
500511
payload.copyable_union.as_list_optional_tensor_ptr != nullptr) {
501-
// for (auto& optional_tensor : toListOptionalTensor()) {
502-
for (auto& optional_tensor :
503-
payload.copyable_union.as_list_optional_tensor_ptr->get()) {
504-
optional_tensor.~optional();
505-
}
512+
payload.copyable_union.as_list_optional_tensor_ptr->destroy_elements();
506513
}
507514
}
508515

0 commit comments

Comments
 (0)