From b240b9d30dae43fff8009aceefbd0b0c4b8ec613 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 2 Sep 2016 20:06:31 +0200 Subject: [PATCH 1/5] Blender: fix invalid OnjectCompare op. --- code/BlenderIntermediate.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/code/BlenderIntermediate.h b/code/BlenderIntermediate.h index fc5d7bf57..83b10e870 100644 --- a/code/BlenderIntermediate.h +++ b/code/BlenderIntermediate.h @@ -123,8 +123,7 @@ namespace Blender { struct ObjectCompare { bool operator() (const Object* left, const Object* right) const { - printf( "left: %s, right: %s\n", left->id.name, right->id.name ); - return ::strncmp(left->id.name, right->id.name, strlen( left->id.name ) ) == 0; + return ::strncmp(left->id.name, right->id.name, strlen( left->id.name ) ) == -1; } }; @@ -145,8 +144,7 @@ namespace Blender { struct ObjectCompare { bool operator() (const Object* left, const Object* right) const { - printf( "left: %s, right: %s\n", left->id.name, right->id.name ); - return ::strncmp( left->id.name, right->id.name, strlen( left->id.name ) ) == 0; + return ::strncmp( left->id.name, right->id.name, strlen( left->id.name ) ) == -1; } }; From e51b7d2a6168f02573287cff497ea9630b291d9f Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 2 Sep 2016 20:06:56 +0200 Subject: [PATCH 2/5] IFC: fix possible use after free access bug. --- code/IFCOpenings.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/code/IFCOpenings.cpp b/code/IFCOpenings.cpp index 51ea14314..3197426cf 100644 --- a/code/IFCOpenings.cpp +++ b/code/IFCOpenings.cpp @@ -715,14 +715,14 @@ void FindAdjacentContours(ContourVector::iterator current, const ContourVector& const Contour& mcontour = (*it).contour; for (size_t n = 0; n < ncontour.size(); ++n) { - const IfcVector2& n0 = ncontour[n]; - const IfcVector2& n1 = ncontour[(n+1) % ncontour.size()]; + const IfcVector2 n0 = ncontour[n]; + const IfcVector2 n1 = ncontour[(n+1) % ncontour.size()]; for (size_t m = 0, mend = (is_me ? n : mcontour.size()); m < mend; ++m) { ai_assert(&mcontour != &ncontour || m < n); - const IfcVector2& m0 = mcontour[m]; - const IfcVector2& m1 = mcontour[(m+1) % mcontour.size()]; + const IfcVector2 m0 = mcontour[m]; + const IfcVector2 m1 = mcontour[(m+1) % mcontour.size()]; IfcVector2 isect0, isect1; if (IntersectingLineSegments(n0,n1, m0, m1, isect0, isect1)) { From bcdc79ba73e325510c451d343bec10230cf552f8 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 4 Sep 2016 20:22:04 +0200 Subject: [PATCH 3/5] Fix invalid release of mat + mesh. --- code/IRRMeshLoader.cpp | 49 ++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/code/IRRMeshLoader.cpp b/code/IRRMeshLoader.cpp index 0a2a54e84..567c98188 100644 --- a/code/IRRMeshLoader.cpp +++ b/code/IRRMeshLoader.cpp @@ -116,6 +116,16 @@ const aiImporterDesc* IRRMeshImporter::GetInfo () const return &desc; } +static void releaseMaterial( aiMaterial **mat ) { + delete mat; + *mat = nullptr; +} + +static void releaseMesh( aiMesh **mesh ) { + delete mesh; + *mesh = nullptr; +} + // ------------------------------------------------------------------------------------------------ // Imports the given file into the given scene structure. void IRRMeshImporter::InternReadFile( const std::string& pFile, @@ -135,7 +145,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, std::vector materials; std::vector meshes; materials.reserve (5); - meshes.reserve (5); + meshes.reserve(5); // temporary data - current mesh buffer aiMaterial* curMat = NULL; @@ -159,11 +169,10 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (!ASSIMP_stricmp(reader->getNodeName(),"buffer") && (curMat || curMesh)) { // end of previous buffer. A material and a mesh should be there if ( !curMat || !curMesh) { - DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); - delete curMat; - delete curMesh; - } - else { + DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); + releaseMaterial( &curMat ); + releaseMesh( &curMesh ); + } else { materials.push_back(curMat); meshes.push_back(curMesh); } @@ -183,7 +192,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (!ASSIMP_stricmp(reader->getNodeName(),"material")) { if (curMat) { DefaultLogger::get()->warn("IRRMESH: Only one material description per buffer, please"); - delete curMat;curMat = NULL; + releaseMaterial( &curMat ); } curMat = ParseMaterial(curMatFlags); } @@ -195,17 +204,16 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, // This is possible ... remove the mesh from the list and skip further reading DefaultLogger::get()->warn("IRRMESH: Found mesh with zero vertices"); - delete curMat;curMat = NULL; - - curMesh = NULL; + releaseMaterial( &curMat ); + releaseMesh( &curMesh ); textMeaning = 0; continue; } - curVertices.reserve (num); - curNormals.reserve (num); - curColors.reserve (num); - curUVs.reserve (num); + curVertices.reserve(num); + curNormals.reserve(num); + curColors.reserve(num); + curUVs.reserve(num); // Determine the file format const char* t = reader->getAttributeValueSafe("type"); @@ -240,7 +248,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, vertexFormat = 2; } else if (ASSIMP_stricmp("standard", t)) { - delete curMat; + releaseMaterial( &curMat ); DefaultLogger::get()->warn("IRRMESH: Unknown vertex format"); } else vertexFormat = 0; @@ -248,7 +256,7 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, } else if (!ASSIMP_stricmp(reader->getNodeName(),"indices")) { if (curVertices.empty() && curMat) { - delete curMat; + releaseMaterial( &curMat ); throw DeadlyImportError("IRRMESH: indices must come after vertices"); } @@ -264,10 +272,10 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, DefaultLogger::get()->warn("IRRMESH: Found mesh with zero indices"); // mesh - away - delete curMesh; curMesh = NULL; + releaseMesh( &curMesh ); // material - away - delete curMat;curMat = NULL; + releaseMaterial( &curMat ); textMeaning = 0; continue; @@ -469,7 +477,6 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, break; default: - // GCC complains here ... break; @@ -480,8 +487,8 @@ void IRRMeshImporter::InternReadFile( const std::string& pFile, if (curMat || curMesh) { if ( !curMat || !curMesh) { DefaultLogger::get()->error("IRRMESH: A buffer must contain a mesh and a material"); - delete curMat; - delete curMesh; + releaseMaterial( &curMat ); + releaseMesh( &curMesh ); } else { materials.push_back(curMat); From 9d4d2b2a1c549d4e958c8e0637d483a7f4e948d4 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 4 Sep 2016 20:35:07 +0200 Subject: [PATCH 4/5] ComputeUVMappingprocess: add missing initialization for scalar value. --- code/ComputeUVMappingProcess.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/ComputeUVMappingProcess.cpp b/code/ComputeUVMappingProcess.cpp index 33a6f4edb..ff5142fc0 100644 --- a/code/ComputeUVMappingProcess.cpp +++ b/code/ComputeUVMappingProcess.cpp @@ -437,7 +437,7 @@ void ComputeUVMappingProcess::Execute( aiScene* pScene) } } - unsigned int idx; + unsigned int idx( 99999999 ); // Check whether we have this mapping mode already std::list::iterator it = std::find (mappingStack.begin(),mappingStack.end(), info); From cc860ede66422c84d4a2211d52ad971011c1b0c5 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 4 Sep 2016 20:40:34 +0200 Subject: [PATCH 5/5] Fix coverity findings: fix usage after free. --- code/SMDLoader.cpp | 31 ++++++++++++++++--------------- code/SMDLoader.h | 2 +- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/code/SMDLoader.cpp b/code/SMDLoader.cpp index 7aef7bc56..2db9f15e7 100644 --- a/code/SMDLoader.cpp +++ b/code/SMDLoader.cpp @@ -74,20 +74,22 @@ static const aiImporterDesc desc = { // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer SMDImporter::SMDImporter() - : configFrameID(), - mBuffer(), - pScene(), - iFileSize(), - iSmallestFrame(), - dLengthOfAnim(), - bHasUVs(), - iLineNumber() -{} +: configFrameID(), +mBuffer(), +pScene( nullptr ), +iFileSize( 0 ), +iSmallestFrame( -1 ), +dLengthOfAnim( 0.0 ), +bHasUVs(false ), +iLineNumber(-1) { + // empty +} // ------------------------------------------------------------------------------------------------ // Destructor, private as well -SMDImporter::~SMDImporter() -{} +SMDImporter::~SMDImporter() { + // empty +} // ------------------------------------------------------------------------------------------------ // Returns whether the class can handle the format of the given file. @@ -133,9 +135,8 @@ void SMDImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOS // Allocate storage and copy the contents of the file to a memory buffer this->pScene = pScene; - std::vector buff(iFileSize+1); - TextFileToBuffer(file.get(),buff); - mBuffer = &buff[0]; + mBuffer.resize( iFileSize + 1 ); + TextFileToBuffer(file.get(), mBuffer ); iSmallestFrame = (1 << 31); bHasUVs = true; @@ -694,7 +695,7 @@ void SMDImporter::CreateOutputMaterials() // Parse the file void SMDImporter::ParseFile() { - const char* szCurrent = mBuffer; + const char* szCurrent = &mBuffer[0]; // read line per line ... for ( ;; ) diff --git a/code/SMDLoader.h b/code/SMDLoader.h index 0b069cc25..96ea1d195 100644 --- a/code/SMDLoader.h +++ b/code/SMDLoader.h @@ -372,7 +372,7 @@ private: unsigned int configFrameID; /** Buffer to hold the loaded file */ - const char* mBuffer; + std::vector mBuffer; /** Output scene to be filled */