Fix Array Access Safety issues (#877)

Summary: Pull Request resolved: #877

Reviewed By: cstollmeta

Differential Revision: D89006772

fbshipit-source-id: 8a922058260d74e6a27d9406808b23d9d6ddbbfa

Author: jeongseok-meta

momentum/character/character_utility.cpp

@@ -655,7 +655,7 @@ Character removeJoints(const Character& character, std::span<const size_t> joint
         if (srcToResultJointsWithParents[iSrcJoint] != kInvalidIndex) {
           break;
         }
-        srcParent = srcSkeleton.joints[srcParent].parent;
+        srcParent = srcSkeleton.joints.at(srcParent).parent;
       }
     }
 

momentum/io/fbx/fbx_io.cpp

@@ -361,6 +361,10 @@ std::vector<::fbxsdk::FbxNode*> createMarkerNodes(
 
       // Add timestamp and position for this marker
       const auto& index = markerNameToIndex.at(marker.name);
+      MT_THROW_IF(
+          index >= timestamps.size() || index >= markerPositions.size(),
+          "Marker index {} exceeds container size",
+          index);
       timestamps[index].push_back(timestamp);
       markerPositions[index].push_back(marker.pos);
     }
@@ -435,6 +439,11 @@ std::vector<::fbxsdk::FbxNode*> createMarkerNodes(
       curveZ->KeyModifyBegin();
 
       for (size_t k = 0; k < timestamps[j].size(); ++k) {
+        MT_THROW_IF(
+            j >= markerPositions.size() || k >= markerPositions[j].size(),
+            "Marker position index out of bounds: j={}, k={}",
+            j,
+            k);
         ::fbxsdk::FbxTime time;
         time.SetSecondDouble(timestamps[j][k]);
 

momentum/io/fbx/openfbx_loader.cpp

@@ -680,6 +680,7 @@ void parseSkinnedModel(
         const auto indexV = shape->getIndices();
         MT_CHECK(numIndices == numVertices);
         for (int v = 0; v < numVertices; ++v) {
+          MT_THROW_IF(v >= numIndices, "Vertex index {} exceeds numIndices {}", v, numIndices);
           shapes(vertexOffset * 3 + indexV[v] * 3 + 0, currentShapes + shapeIndex) = shapeV[v].x;
           shapes(vertexOffset * 3 + indexV[v] * 3 + 1, currentShapes + shapeIndex) = shapeV[v].y;
           shapes(vertexOffset * 3 + indexV[v] * 3 + 2, currentShapes + shapeIndex) = shapeV[v].z;

momentum/io/gltf/gltf_builder.cpp

@@ -322,6 +322,10 @@ void addActorAnimationToModel(
 
       // get index
       const auto& index = markerNameToIndex.at(marker.name);
+      MT_THROW_IF(
+          index >= timestamps.size() || index >= markerPositions.size(),
+          "Marker index {} exceeds container size",
+          index);
       timestamps[index].push_back(timestamp);
       markerPositions[index].push_back(fromMomentumVec3f(marker.pos.cast<float>()));
     }
@@ -384,6 +388,11 @@ size_t addMeshToModel(
     const auto nodeIdx = model.nodes.size();
     model.nodes.emplace_back();
     auto& node = model.nodes.back();
+    MT_THROW_IF(
+        rootNodeIdx >= model.nodes.size(),
+        "Root node index {} exceeds model.nodes size {}",
+        rootNodeIdx,
+        model.nodes.size());
     model.nodes[rootNodeIdx].children.push_back(nodeIdx);
 
     // create model mesh

momentum/io/gltf/gltf_io.cpp

@@ -440,6 +440,7 @@ addMesh(const fx::gltf::Document& model, const fx::gltf::Primitive& primitive, M
     MT_CHECK(fidxDense.size() % 3 == 0, "{} % 3 = {}", fidxDense.size(), fidxDense.size() % 3);
     texfaces.resize(fidxDense.size() / 3);
     if (!fidxDense.empty()) {
+      MT_THROW_IF(texfaces.empty(), "texfaces is empty but fidxDense is not empty");
       std::copy_n(fidxDense.data(), fidxDense.size(), &texfaces[0][0]);
     }
   } else {

momentum/io/marker/trc_io.cpp

@@ -7,6 +7,7 @@
 
 #include "momentum/io/marker/trc_io.h"
 
+#include "momentum/common/exception.h"
 #include "momentum/common/string.h"
 #include "momentum/io/common/stream_utils.h"
 #include "momentum/io/marker/conversions.h"
@@ -76,6 +77,8 @@ MarkerSequence loadTrc(const std::string& filename, UpVector up) {
     // go over all markers
     for (size_t i = 0; i < numMarkers; i++) {
       const size_t pos = i * 3 + 2;
+      MT_THROW_IF(
+          i >= markerNames.size() || pos + 2 >= tokens.size(), "Marker index {} out of bounds", i);
       markers[i].name = markerNames[i];
       if (tokens[pos + 0].empty() || tokens[pos + 1].empty() || tokens[pos + 2].empty()) {
         markers[i].occluded = true;

pymomentum/renderer/momentum_render.cpp

@@ -383,9 +383,10 @@ Eigen::Matrix<int, Eigen::Dynamic, 3, Eigen::RowMajor> triangulate(
   const size_t numTris = triangleIndices.size() / 3;
   Eigen::Matrix<int, Eigen::Dynamic, 3, Eigen::RowMajor> result(numTris, 3);
   for (size_t iTri = 0; iTri < numTris; iTri++) {
-    result(iTri, 0) = triangleIndices[iTri * 3 + 0];
-    result(iTri, 1) = triangleIndices[iTri * 3 + 1];
-    result(iTri, 2) = triangleIndices[iTri * 3 + 2];
+    const size_t baseIdx = iTri * 3;
+    result(iTri, 0) = triangleIndices.at(baseIdx + 0);
+    result(iTri, 1) = triangleIndices.at(baseIdx + 1);
+    result(iTri, 2) = triangleIndices.at(baseIdx + 2);
   }
   return result;
 }

pymomentum/renderer/software_rasterizer.cpp

@@ -14,6 +14,7 @@
 #include <momentum/character/linear_skinning.h>
 #include <momentum/character/skeleton.h>
 #include <momentum/character/skeleton_state.h>
+#include <momentum/common/exception.h>
 #include <momentum/math/fwd.h>
 #include <momentum/math/mesh.h>
 
@@ -911,6 +912,11 @@ void rasterizeSkeleton(
 
     size_t grandparent = character.skeleton.joints[iJoint].parent;
     while (grandparent != momentum::kInvalidIndex && !activeJoints[grandparent]) {
+      MT_THROW_IF(
+          grandparent >= character.skeleton.joints.size(),
+          "Grandparent joint index {} exceeds skeleton size {}",
+          grandparent,
+          character.skeleton.joints.size());
       grandparent = character.skeleton.joints[grandparent].parent;
     }
 

pymomentum/solver/momentum_ik.cpp

@@ -473,6 +473,11 @@ variable_list IKProblemAutogradFunction<T, IKFunction>::forward(
   // cannot save a vector of custom enum type.
   std::vector<int64_t> activatedErrorTypes(activeErrorFunctions.size());
   for (size_t i = 0; i < activeErrorFunctions.size(); ++i) {
+    MT_THROW_IF(
+        i >= activatedErrorTypes.size(),
+        "Index {} exceeds activatedErrorTypes size {}",
+        i,
+        activatedErrorTypes.size());
     activatedErrorTypes[i] = static_cast<int>(activeErrorFunctions[i]);
   }
   ctx->saved_data["activeErrorFunctions"] = activatedErrorTypes;

pymomentum/tensor_ik/tensor_ik_utility.cpp

@@ -163,6 +163,11 @@ std::vector<std::shared_ptr<momentum::SkeletonErrorFunctionT<T>>> buildMomentumE
     result.push_back(errf->createErrorFunction(character, iBatch, jFrame));
     // weightsMap maps error type order in the enum to order in input
     // errorFunctionWeights.
+    MT_THROW_IF(
+        iErr >= weightsMap.size(),
+        "Error function index {} exceeds weightsMap size {}",
+        iErr,
+        weightsMap.size());
     T weight = weightsMap[iErr] < 0 ? T(0) : toEigenMap<T>(weights_cur)[weightsMap[iErr]];
     result.back()->setWeight(weight);
   }
@@ -207,6 +212,11 @@ std::unique_ptr<momentum::SequenceSolverFunctionT<T>> buildSequenceSolverFunctio
       auto errf_momentum = errf->createErrorFunction(character, iBatch, jFrame);
       // weightsMap maps error type order in the enum to order in input
       // errorFunctionWeights.
+      MT_THROW_IF(
+          iErr >= weightsMap.size(),
+          "Error function index {} exceeds weightsMap size {}",
+          iErr,
+          weightsMap.size());
       T weight = weightsMap[iErr] < 0 ? T(0) : toEigenMap<T>(weights_cur)[weightsMap[iErr]];
       errf_momentum->setWeight(weight);
 
@@ -238,6 +248,11 @@ std::vector<ErrorFunctionInput<T>> buildErrorFunctionInputs(
         continue;
       }
 
+      MT_THROW_IF(
+          iErrorFunction >= weightsMap.size(),
+          "Error function index {} exceeds weightsMap size {}",
+          iErrorFunction,
+          weightsMap.size());
       if (weightsMap[iErrorFunction] < 0) {
         continue;
       }