From c91d7de572197b1fb68f2fabca36bf8f60fc7461 Mon Sep 17 00:00:00 2001 From: Dave Bort Date: Tue, 14 Jan 2025 11:10:04 -0800 Subject: [PATCH] Validate value indices when deserializing list types (#7637) Summary: List types contain lists of indices into the method's `values_` list. Ensure that we don't overflow the list when those indices are corrupt. This should guard: - IntList - OptionalTensorList - TensorList The other scalar list types do not do the unboxing that IntList does, so shouldn't run into this problem. Differential Revision: D68127899 --- runtime/executor/method.cpp | 12 +++++++++++- runtime/executor/tensor_parser.h | 17 ++++++++++++----- runtime/executor/tensor_parser_exec_aten.cpp | 17 ++++++++++++----- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/runtime/executor/method.cpp b/runtime/executor/method.cpp index 674af6d69f9..c9563d3ae51 100644 --- a/runtime/executor/method.cpp +++ b/runtime/executor/method.cpp @@ -351,7 +351,15 @@ Error Method::parse_values() { // initialize boxed list for (size_t j = 0; j < items->size(); j++) { - evalp_list[j] = &values_[static_cast(items->Get(j))]; + auto value_index = items->Get(j); + ET_CHECK_OR_RETURN_ERROR( + value_index >= 0 && value_index < n_value, + InvalidProgram, + "Invalid value index %" PRId64 " for IntList %zu index %zu", + value_index, + i, + j); + evalp_list[j] = &values_[static_cast(value_index)]; } new (&values_[i]) EValue( BoxedEvalueList(evalp_list, int_list, items->size())); @@ -411,6 +419,7 @@ Error Method::parse_values() { auto tensors = deserialization::parseTensorList( static_cast(val)->items(), values_, + n_value, // The size of the full array. memory_manager_); if (!tensors.ok()) { ET_LOG( @@ -430,6 +439,7 @@ Error Method::parse_values() { val) ->items(), values_, + n_value, // The size of the full array. memory_manager_); if (!tensors.ok()) { ET_LOG( diff --git a/runtime/executor/tensor_parser.h b/runtime/executor/tensor_parser.h index d0a818a721c..6b593afe7c0 100644 --- a/runtime/executor/tensor_parser.h +++ b/runtime/executor/tensor_parser.h @@ -25,7 +25,8 @@ ET_NODISCARD Result parseTensor( ET_NODISCARD Result> parseTensorList( const flatbuffers::Vector* tensor_indices, - EValue* values_, + EValue* values, + size_t values_len, MemoryManager* memory_manager); // Deserializes a List of optional type. The code here is the same between all @@ -35,7 +36,8 @@ template ET_NODISCARD Result>> parseListOptionalType( const flatbuffers::Vector* value_indices, - EValue* values_, + EValue* values, + size_t values_len, MemoryManager* memory_manager) { auto* evalp_list = memory_manager->method_allocator()->allocateList( value_indices->size()); @@ -55,7 +57,7 @@ parseListOptionalType( // already allocated) and stick it in the list. for (int32_t index : *value_indices) { // Lists of objects are stored in fbb as list[int] where the ints are - // indices into values_. Currently serialization is deciding if they want to + // indices into values. Currently serialization is deciding if they want to // put -1 for serialized None type indices, or give us a valid index to a // serialized None. We support either for now. // Placement new as the list elements are not initialized, so calling @@ -68,9 +70,14 @@ parseListOptionalType( // TODO(T161156879): do something less hacky here. evalp_list[output_idx] = nullptr; } else { + ET_CHECK_OR_RETURN_ERROR( + index >= 0 && index < values_len, + InvalidProgram, + "Invalid value index %" PRId32 " for ListOptional", + index); new (&optional_tensor_list[output_idx]) - executorch::aten::optional(values_[index].toOptional()); - evalp_list[output_idx] = &values_[static_cast(index)]; + executorch::aten::optional(values[index].toOptional()); + evalp_list[output_idx] = &values[static_cast(index)]; } output_idx++; } diff --git a/runtime/executor/tensor_parser_exec_aten.cpp b/runtime/executor/tensor_parser_exec_aten.cpp index 23101e5a1ca..a3844a34b43 100644 --- a/runtime/executor/tensor_parser_exec_aten.cpp +++ b/runtime/executor/tensor_parser_exec_aten.cpp @@ -70,7 +70,8 @@ ET_NODISCARD Result getMemPlannedPtr( ET_NODISCARD Result> parseTensorList( const flatbuffers::Vector* tensor_indices, - EValue* values_, + EValue* values, + size_t values_len, MemoryManager* memory_manager) { EXECUTORCH_SCOPE_PROF("TensorParser::parseTensorList"); @@ -90,11 +91,17 @@ ET_NODISCARD Result> parseTensorList( // already allocated) and stick it in the list. size_t output_idx = 0; for (int32_t tensor_index : *tensor_indices) { + ET_CHECK_OR_RETURN_ERROR( + tensor_index >= 0 && tensor_index < values_len, + InvalidProgram, + "Invalid value index %" PRId32 " for TensorList", + tensor_index); + // Placement new as the list elements are not initialized, so calling - // copy assignment is not defined if its non trivial. - new (&tensor_list[output_idx]) exec_aten::Tensor( - values_[static_cast(tensor_index)].toTensor()); - evalp_list[output_idx] = &values_[static_cast(tensor_index)]; + // copy assignment is not defined if it's non trivial. + new (&tensor_list[output_idx]) + exec_aten::Tensor(values[static_cast(tensor_index)].toTensor()); + evalp_list[output_idx] = &values[static_cast(tensor_index)]; output_idx++; }