From add165c4a19a77e96017e249b8028c2ff09695c6 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Wed, 24 Mar 2021 10:55:40 +0000 Subject: [PATCH 1/2] Check target sizes to avoid reading beyond allocation --- code/AssetLib/glTF2/glTF2Importer.cpp | 52 ++++++++++++++++----------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index 8442c4958..0574c25da 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -536,35 +536,47 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { Mesh::Primitive::Target &target = targets[i]; if (needPositions) { - aiVector3D *positionDiff = nullptr; - target.position[0]->ExtractData(positionDiff); - for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; vertexId++) { - aiAnimMesh.mVertices[vertexId] += positionDiff[vertexId]; + if (target.position[0]->count != aim->mNumVertices) { + DefaultLogger::get()->warn("Positions of target in mesh \"" + mesh.name + "\" does not match the vertex count"); + } else { + aiVector3D *positionDiff = nullptr; + target.position[0]->ExtractData(positionDiff); + for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; vertexId++) { + aiAnimMesh.mVertices[vertexId] += positionDiff[vertexId]; + } + delete[] positionDiff; } - delete[] positionDiff; } if (needNormals) { - aiVector3D *normalDiff = nullptr; - target.normal[0]->ExtractData(normalDiff); - for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; vertexId++) { - aiAnimMesh.mNormals[vertexId] += normalDiff[vertexId]; + if (target.normal[0]->count != aim->mNumVertices) { + DefaultLogger::get()->warn("Normals of target in mesh \"" + mesh.name + "\" does not match the vertex count"); + } else { + aiVector3D *normalDiff = nullptr; + target.normal[0]->ExtractData(normalDiff); + for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; vertexId++) { + aiAnimMesh.mNormals[vertexId] += normalDiff[vertexId]; + } + delete[] normalDiff; } - delete[] normalDiff; } if (needTangents) { - Tangent *tangent = nullptr; - attr.tangent[0]->ExtractData(tangent); + if (target.tangent[0]->count != aim->mNumVertices) { + DefaultLogger::get()->warn("Tangents of target in mesh \"" + mesh.name + "\" does not match the vertex count"); + } else { + Tangent *tangent = nullptr; + attr.tangent[0]->ExtractData(tangent); - aiVector3D *tangentDiff = nullptr; - target.tangent[0]->ExtractData(tangentDiff); + aiVector3D *tangentDiff = nullptr; + target.tangent[0]->ExtractData(tangentDiff); - for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; ++vertexId) { - tangent[vertexId].xyz += tangentDiff[vertexId]; - aiAnimMesh.mTangents[vertexId] = tangent[vertexId].xyz; - aiAnimMesh.mBitangents[vertexId] = (aiAnimMesh.mNormals[vertexId] ^ tangent[vertexId].xyz) * tangent[vertexId].w; + for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; ++vertexId) { + tangent[vertexId].xyz += tangentDiff[vertexId]; + aiAnimMesh.mTangents[vertexId] = tangent[vertexId].xyz; + aiAnimMesh.mBitangents[vertexId] = (aiAnimMesh.mNormals[vertexId] ^ tangent[vertexId].xyz) * tangent[vertexId].w; + } + delete[] tangent; + delete[] tangentDiff; } - delete[] tangent; - delete[] tangentDiff; } if (mesh.weights.size() > i) { aiAnimMesh.mWeight = mesh.weights[i]; From d94ddd32b1a902bf2fbbd3bd546b02cb89cfc166 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Wed, 24 Mar 2021 11:04:39 +0000 Subject: [PATCH 2/2] Extra check. Better logging. --- code/AssetLib/glTF2/glTF2Importer.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index 0574c25da..dca6fcb2d 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -527,8 +527,8 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { std::fill(aim->mAnimMeshes, aim->mAnimMeshes + aim->mNumAnimMeshes, nullptr); for (size_t i = 0; i < targets.size(); i++) { bool needPositions = targets[i].position.size() > 0; - bool needNormals = targets[i].normal.size() > 0; - bool needTangents = targets[i].tangent.size() > 0; + bool needNormals = (targets[i].normal.size() > 0) && aim->HasNormals(); + bool needTangents = (targets[i].tangent.size() > 0) && aim->HasTangentsAndBitangents(); // GLTF morph does not support colors and texCoords aim->mAnimMeshes[i] = aiCreateAnimMesh(aim, needPositions, needNormals, needTangents, false, false); @@ -537,7 +537,7 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { if (needPositions) { if (target.position[0]->count != aim->mNumVertices) { - DefaultLogger::get()->warn("Positions of target in mesh \"" + mesh.name + "\" does not match the vertex count"); + ASSIMP_LOG_WARN_F("Positions of target ", i, " in mesh \"", mesh.name, "\" does not match the vertex count"); } else { aiVector3D *positionDiff = nullptr; target.position[0]->ExtractData(positionDiff); @@ -549,7 +549,7 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { } if (needNormals) { if (target.normal[0]->count != aim->mNumVertices) { - DefaultLogger::get()->warn("Normals of target in mesh \"" + mesh.name + "\" does not match the vertex count"); + ASSIMP_LOG_WARN_F("Normals of target ", i, " in mesh \"", mesh.name, "\" does not match the vertex count"); } else { aiVector3D *normalDiff = nullptr; target.normal[0]->ExtractData(normalDiff); @@ -561,7 +561,7 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { } if (needTangents) { if (target.tangent[0]->count != aim->mNumVertices) { - DefaultLogger::get()->warn("Tangents of target in mesh \"" + mesh.name + "\" does not match the vertex count"); + ASSIMP_LOG_WARN_F("Tangents of target ", i, " in mesh \"", mesh.name, "\" does not match the vertex count"); } else { Tangent *tangent = nullptr; attr.tangent[0]->ExtractData(tangent);