From fba7ede639e746e282448cf238cd7d59790dec61 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 28 Oct 2015 13:30:56 +0200 Subject: [PATCH 1/3] OFFLoader: Don't use assignments as expressions --- code/OFFLoader.cpp | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/code/OFFLoader.cpp b/code/OFFLoader.cpp index d5c23dcfa..d801a2eb0 100644 --- a/code/OFFLoader.cpp +++ b/code/OFFLoader.cpp @@ -138,9 +138,15 @@ void OFFImporter::InternReadFile( const std::string& pFile, throw DeadlyImportError("OFF: There are no valid faces"); } - pScene->mMeshes = new aiMesh*[ pScene->mNumMeshes = 1 ]; - aiMesh* mesh = pScene->mMeshes[0] = new aiMesh(); - aiFace* faces = mesh->mFaces = new aiFace [mesh->mNumFaces = numFaces]; + pScene->mNumMeshes = 1; + pScene->mMeshes = new aiMesh*[ pScene->mNumMeshes ]; + + aiMesh* mesh = new aiMesh(); + pScene->mMeshes[0] = mesh; + + mesh->mNumFaces = numFaces; + aiFace* faces = new aiFace [mesh->mNumFaces]; + mesh->mFaces = faces; std::vector tempPositions(numVertices); @@ -171,7 +177,8 @@ void OFFImporter::InternReadFile( const std::string& pFile, break; } sz = line;SkipSpaces(&sz); - if(!(faces->mNumIndices = strtoul10(sz,&sz)) || faces->mNumIndices > 9) + faces->mNumIndices = strtoul10(sz,&sz); + if(!(faces->mNumIndices) || faces->mNumIndices > 9) { DefaultLogger::get()->error("OFF: Faces with zero indices aren't allowed"); --mesh->mNumFaces; @@ -185,24 +192,28 @@ void OFFImporter::InternReadFile( const std::string& pFile, throw DeadlyImportError("OFF: There are no valid faces"); // allocate storage for the output vertices - aiVector3D* verts = mesh->mVertices = new aiVector3D[mesh->mNumVertices]; + aiVector3D* verts = new aiVector3D[mesh->mNumVertices]; + mesh->mVertices = verts; // second: now parse all face indices - buffer = old;faces = mesh->mFaces; + buffer = old; + faces = mesh->mFaces; for (unsigned int i = 0, p = 0; i< mesh->mNumFaces;) { if(!GetNextLine(buffer,line))break; unsigned int idx; sz = line;SkipSpaces(&sz); - if(!(idx = strtoul10(sz,&sz)) || idx > 9) + idx = strtoul10(sz,&sz); + if(!(idx) || idx > 9) continue; faces->mIndices = new unsigned int [faces->mNumIndices]; for (unsigned int m = 0; m < faces->mNumIndices;++m) { SkipSpaces(&sz); - if ((idx = strtoul10(sz,&sz)) >= numVertices) + idx = strtoul10(sz,&sz); + if ((idx) >= numVertices) { DefaultLogger::get()->error("OFF: Vertex index is out of range"); idx = numVertices-1; @@ -217,11 +228,13 @@ void OFFImporter::InternReadFile( const std::string& pFile, // generate the output node graph pScene->mRootNode = new aiNode(); pScene->mRootNode->mName.Set(""); - pScene->mRootNode->mMeshes = new unsigned int [pScene->mRootNode->mNumMeshes = 1]; + pScene->mRootNode->mNumMeshes = 1; + pScene->mRootNode->mMeshes = new unsigned int [pScene->mRootNode->mNumMeshes]; pScene->mRootNode->mMeshes[0] = 0; // generate a default material - pScene->mMaterials = new aiMaterial*[pScene->mNumMaterials = 1]; + pScene->mNumMaterials = 1; + pScene->mMaterials = new aiMaterial*[pScene->mNumMaterials]; aiMaterial* pcMat = new aiMaterial(); aiColor4D clr(0.6f,0.6f,0.6f,1.0f); From 7a5bc6eca34dd6a58f230acdada74248cfc95e5a Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 28 Oct 2015 14:10:18 +0200 Subject: [PATCH 2/3] OFFLoader: Use a temporary vector to store vertices instead of a raw array Prevents crash on certain malformed inputs but they still cause a validation failure. --- code/OFFLoader.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/code/OFFLoader.cpp b/code/OFFLoader.cpp index d801a2eb0..ea563ca9e 100644 --- a/code/OFFLoader.cpp +++ b/code/OFFLoader.cpp @@ -192,8 +192,8 @@ void OFFImporter::InternReadFile( const std::string& pFile, throw DeadlyImportError("OFF: There are no valid faces"); // allocate storage for the output vertices - aiVector3D* verts = new aiVector3D[mesh->mNumVertices]; - mesh->mVertices = verts; + std::vector verts; + verts.reserve(mesh->mNumVertices); // second: now parse all face indices buffer = old; @@ -219,12 +219,14 @@ void OFFImporter::InternReadFile( const std::string& pFile, idx = numVertices-1; } faces->mIndices[m] = p++; - *verts++ = tempPositions[idx]; + verts.push_back(tempPositions[idx]); } ++i; ++faces; } + mesh->mVertices = new aiVector3D[verts.size()]; + memcpy(mesh->mVertices, &verts[0], verts.size() * sizeof(aiVector3D)); // generate the output node graph pScene->mRootNode = new aiNode(); pScene->mRootNode->mName.Set(""); From 9825d07764b9934ab2a88fa05f9f5edba4beb8e2 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 28 Oct 2015 14:20:13 +0200 Subject: [PATCH 3/3] OFFLoader: Throw error on certain invalid files instead of failing validation later --- code/OFFLoader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/OFFLoader.cpp b/code/OFFLoader.cpp index ea563ca9e..7e4429c88 100644 --- a/code/OFFLoader.cpp +++ b/code/OFFLoader.cpp @@ -225,6 +225,9 @@ void OFFImporter::InternReadFile( const std::string& pFile, ++faces; } + if (mesh->mNumVertices != verts.size()) { + throw DeadlyImportError("OFF: Vertex count mismatch"); + } mesh->mVertices = new aiVector3D[verts.size()]; memcpy(mesh->mVertices, &verts[0], verts.size() * sizeof(aiVector3D)); // generate the output node graph