Validate value indices when deserializing list types

Differential Revision: D68127899

Pull Request resolved: #7637

Author: dbort

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<size_t>(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<size_t>(value_index)];
         }
         new (&values_[i]) EValue(
             BoxedEvalueList<int64_t>(evalp_list, int_list, items->size()));
@@ -411,6 +419,7 @@ Error Method::parse_values() {
         auto tensors = deserialization::parseTensorList(
             static_cast<const executorch_flatbuffer::TensorList*>(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(

runtime/executor/tensor_parser.h

@@ -25,7 +25,8 @@ ET_NODISCARD Result<executorch::aten::Tensor> parseTensor(
 
 ET_NODISCARD Result<BoxedEvalueList<executorch::aten::Tensor>> parseTensorList(
     const flatbuffers::Vector<int32_t>* 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 <typename T>
 ET_NODISCARD Result<BoxedEvalueList<executorch::aten::optional<T>>>
 parseListOptionalType(
     const flatbuffers::Vector<int32_t>* value_indices,
-    EValue* values_,
+    EValue* values,
+    size_t values_len,
     MemoryManager* memory_manager) {
   auto* evalp_list = memory_manager->method_allocator()->allocateList<EValue*>(
       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<T>(values_[index].toOptional<T>());
-      evalp_list[output_idx] = &values_[static_cast<size_t>(index)];
+          executorch::aten::optional<T>(values[index].toOptional<T>());
+      evalp_list[output_idx] = &values[static_cast<size_t>(index)];
     }
     output_idx++;
   }

runtime/executor/tensor_parser_exec_aten.cpp

@@ -70,7 +70,8 @@ ET_NODISCARD Result<void*> getMemPlannedPtr(
 
 ET_NODISCARD Result<BoxedEvalueList<exec_aten::Tensor>> parseTensorList(
     const flatbuffers::Vector<int32_t>* 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<BoxedEvalueList<exec_aten::Tensor>> 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<size_t>(tensor_index)].toTensor());
-    evalp_list[output_idx] = &values_[static_cast<size_t>(tensor_index)];
+    // copy assignment is not defined if it's non trivial.
+    new (&tensor_list[output_idx])
+        exec_aten::Tensor(values[static_cast<size_t>(tensor_index)].toTensor());
+    evalp_list[output_idx] = &values[static_cast<size_t>(tensor_index)];
     output_idx++;
   }