From 9f9774403d9800cf1640ba050a786aef578438dd Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:03:06 +0300 Subject: [PATCH 1/7] Remove assertion when too long message attempted Since these can be caused by malformed input files assert is the wrong thing. --- code/DefaultLogger.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/code/DefaultLogger.cpp b/code/DefaultLogger.cpp index 1b9167c55..1e5b8b9c5 100644 --- a/code/DefaultLogger.cpp +++ b/code/DefaultLogger.cpp @@ -165,7 +165,6 @@ void Logger::debug(const char* message) { // sometimes importers will include data from the input file // (i.e. node names) in their messages. if (strlen(message)>MAX_LOG_MESSAGE_LENGTH) { - ai_assert(false); return; } return OnDebug(message); @@ -176,7 +175,6 @@ void Logger::info(const char* message) { // SECURITY FIX: see above if (strlen(message)>MAX_LOG_MESSAGE_LENGTH) { - ai_assert(false); return; } return OnInfo(message); @@ -187,7 +185,6 @@ void Logger::warn(const char* message) { // SECURITY FIX: see above if (strlen(message)>MAX_LOG_MESSAGE_LENGTH) { - ai_assert(false); return; } return OnWarn(message); @@ -198,7 +195,6 @@ void Logger::error(const char* message) { // SECURITY FIX: see above if (strlen(message)>MAX_LOG_MESSAGE_LENGTH) { - ai_assert(false); return; } return OnError(message); From 8cdf9467c561061a11529ba8e247cba86d53f15f Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:09:59 +0300 Subject: [PATCH 2/7] MD3: Fix assertion failures when filename doesn't contain '.' --- code/MD3Loader.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/code/MD3Loader.cpp b/code/MD3Loader.cpp index 49cecd1bf..dd5615e7a 100644 --- a/code/MD3Loader.cpp +++ b/code/MD3Loader.cpp @@ -471,6 +471,9 @@ void MD3Importer::ReadSkin(Q3Shader::SkinData& fill) const std::string::size_type s = filename.find_last_of('_'); if (s == std::string::npos) { s = filename.find_last_of('.'); + if (s == std::string::npos) { + s = filename.size(); + } } ai_assert(s != std::string::npos); @@ -532,7 +535,9 @@ bool MD3Importer::ReadMultipartFile() { // check whether the file name contains a common postfix, e.g lower_2.md3 std::string::size_type s = filename.find_last_of('_'), t = filename.find_last_of('.'); - ai_assert(t != std::string::npos); + + if (t == std::string::npos) + t = filename.size(); if (s == std::string::npos) s = t; From 4c28f31f43ad18586d78652fa96d782e1e509794 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:11:53 +0300 Subject: [PATCH 3/7] X: Throw error when scene contains no root node Otherwise MakeLeftHandedProcess will crash. --- code/XFileImporter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/XFileImporter.cpp b/code/XFileImporter.cpp index 6f16d7451..75932a17a 100644 --- a/code/XFileImporter.cpp +++ b/code/XFileImporter.cpp @@ -156,6 +156,10 @@ void XFileImporter::CreateDataRepresentationFromImport( aiScene* pScene, XFile:: CreateMeshes( pScene, pScene->mRootNode, pData->mGlobalMeshes); } + if (!pScene->mRootNode) { + throw DeadlyImportError( "No root node" ); + } + // Convert everything to OpenGL space... it's the same operation as the conversion back, so we can reuse the step directly MakeLeftHandedProcess convertProcess; convertProcess.Execute( pScene); From 3e728e80ebe379951e801b8d85e58c09c93da7e4 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:12:46 +0300 Subject: [PATCH 4/7] Obj: Change asserts to exceptions These can be triggered by malformed input file so they can't be assertions. --- code/ObjFileImporter.cpp | 4 +++- code/ObjFileParser.cpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index 8f201aa5a..d803ad6f8 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -424,7 +424,9 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, pMesh->mTextureCoords[ 0 ][ newIndex ] = aiVector3D( coord3d.x, coord3d.y, coord3d.z ); } - ai_assert( pMesh->mNumVertices > newIndex ); + if ( pMesh->mNumVertices <= newIndex ) { + throw DeadlyImportError("OBJ: bad vertex index"); + } // Get destination face aiFace *pDestFace = &pMesh->mFaces[ outIndex ]; diff --git a/code/ObjFileParser.cpp b/code/ObjFileParser.cpp index c4dff0ba0..d6186849f 100644 --- a/code/ObjFileParser.cpp +++ b/code/ObjFileParser.cpp @@ -259,7 +259,7 @@ void ObjFileParser::getVector( std::vector &point3d_array ) { copyNextWord( m_buffer, BUFFERSIZE ); z = ( float ) fast_atof( m_buffer ); } else { - ai_assert( !"Invalid number of components" ); + throw DeadlyImportError( "OBJ: Invalid number of components" ); } point3d_array.push_back( aiVector3D( x, y, z ) ); m_DataIt = skipLine( m_DataIt, m_DataItEnd, m_uiLine ); From 16c57ab1d3e507ca37e89836c0e4aba6053cc161 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:22:06 +0300 Subject: [PATCH 5/7] AC3D: Throw exception on too many vertices instead of crashing --- code/ACLoader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/ACLoader.cpp b/code/ACLoader.cpp index fc00316bf..fccec4e61 100644 --- a/code/ACLoader.cpp +++ b/code/ACLoader.cpp @@ -598,6 +598,9 @@ aiNode* AC3DImporter::ConvertObjectSection(Object& object, face.mIndices[i] = cur++; // copy vertex positions + if ((vertices - mesh->mVertices) >= mesh->mNumVertices) { + throw DeadlyImportError("AC3D: Invalid number of vertices"); + } *vertices = object.vertices[entry.first] + object.translation; From bf5c9413f980777d912c7abcc4e3a5e949a91cdb Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:22:46 +0300 Subject: [PATCH 6/7] AC3D: Throw exception when encountering a bad vertex index --- code/ACLoader.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/ACLoader.cpp b/code/ACLoader.cpp index fccec4e61..d3f45918a 100644 --- a/code/ACLoader.cpp +++ b/code/ACLoader.cpp @@ -632,6 +632,10 @@ aiNode* AC3DImporter::ConvertObjectSection(Object& object, face.mIndices[1] = cur++; // copy vertex positions + if (it2 == (*it).entries.end() ) { + throw DeadlyImportError("AC3D: Bad line"); + } + ai_assert((*it2).first < object.vertices.size()); *vertices++ = object.vertices[(*it2).first]; // copy texture coordinates From 681c32d5ecb34dbd4792f93fa7f238dd5bd91c37 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Wed, 1 Apr 2015 16:23:28 +0300 Subject: [PATCH 7/7] AC3D: Throw DeadlyImportError when too many vertices instead of out-of-memory exception Valgrind can't throw an exception when running out of memory. The program wil just crash. This fixes it in some cases but not all. --- code/ACLoader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/ACLoader.cpp b/code/ACLoader.cpp index d3f45918a..47f0b8622 100644 --- a/code/ACLoader.cpp +++ b/code/ACLoader.cpp @@ -274,6 +274,9 @@ void AC3DImporter::LoadObjectSection(std::vector& objects) SkipSpaces(&buffer); unsigned int t = strtoul10(buffer,&buffer); + if (t >= std::numeric_limits::max() / sizeof(aiVector3D)) { + throw DeadlyImportError("AC3D: Too many vertices, would run out of memory"); + } obj.vertices.reserve(t); for (unsigned int i = 0; i < t;++i) {