From 4cc716a0f5198f9ad9e32d79de14db535ae51c12 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 11 Aug 2015 14:32:26 +0300 Subject: [PATCH 1/5] MDL: Fix read past end of buffer with malformed input --- code/MDLLoader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/MDLLoader.cpp b/code/MDLLoader.cpp index 60c92661b..c5b8d63c3 100644 --- a/code/MDLLoader.cpp +++ b/code/MDLLoader.cpp @@ -355,6 +355,9 @@ void MDLImporter::InternReadFile_Quake1( ) for (unsigned int i = 0; i < (unsigned int)pcHeader->num_skins;++i) { union{BE_NCONST MDL::Skin* pcSkin;BE_NCONST MDL::GroupSkin* pcGroupSkin;}; + if (szCurrent + sizeof(MDL::Skin) > this->mBuffer + this->iFileSize) { + throw DeadlyImportError("[Quake 1 MDL] Unexpected EOF"); + } pcSkin = (BE_NCONST MDL::Skin*)szCurrent; AI_SWAP4( pcSkin->group ); From d185cea81c669cc0a2330c97a54a669892867599 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 11 Aug 2015 15:44:51 +0300 Subject: [PATCH 2/5] AC3D: Fix read past end of buffer --- code/ACLoader.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/code/ACLoader.cpp b/code/ACLoader.cpp index dac85b8db..642fd5d24 100644 --- a/code/ACLoader.cpp +++ b/code/ACLoader.cpp @@ -89,6 +89,9 @@ static const aiImporterDesc desc = { // ------------------------------------------------------------------------------------------------ // read a string (may be enclosed in double quotation marks). buffer must point to " #define AI_AC_GET_STRING(out) \ + if (*buffer == '\0') { \ + throw DeadlyImportError("AC3D: Unexpected EOF in string"); \ + } \ ++buffer; \ const char* sz = buffer; \ while ('\"' != *buffer) \ From e5ddb98dde79530d3bacd83c806d36924d2accf8 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 11 Aug 2015 15:53:16 +0300 Subject: [PATCH 3/5] STL: Fix another read past EOF --- code/STLLoader.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/code/STLLoader.cpp b/code/STLLoader.cpp index 1bf47dcf2..fc6ca0661 100644 --- a/code/STLLoader.cpp +++ b/code/STLLoader.cpp @@ -304,6 +304,9 @@ void STLImporter::LoadASCIIFile() } else { + if (sz[6] == '\0') { + throw DeadlyImportError("STL: unexpected EOF while parsing facet"); + } sz += 7; SkipSpaces(&sz); sz = fast_atoreal_move(sz, (float&)vn->x ); @@ -324,6 +327,9 @@ void STLImporter::LoadASCIIFile() } else { + if (sz[6] == '\0') { + throw DeadlyImportError("STL: unexpected EOF while parsing facet"); + } sz += 7; SkipSpaces(&sz); positionBuffer.push_back(aiVector3D()); From 5575a54466ea54902c837b9846b44a1c89d777fa Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Tue, 11 Aug 2015 16:09:19 +0300 Subject: [PATCH 4/5] Add various checks to avoid either too large or zero-sized memory allocations --- code/ACLoader.cpp | 10 ++++++++++ code/MD3Loader.cpp | 5 +++++ code/ObjFileImporter.cpp | 5 +++++ 3 files changed, 20 insertions(+) diff --git a/code/ACLoader.cpp b/code/ACLoader.cpp index 642fd5d24..8d2fac171 100644 --- a/code/ACLoader.cpp +++ b/code/ACLoader.cpp @@ -587,9 +587,19 @@ aiNode* AC3DImporter::ConvertObjectSection(Object& object, // allocate storage for vertices and normals mesh->mNumFaces = (*cit).first; + if (mesh->mNumFaces == 0) { + throw DeadlyImportError("AC3D: No faces"); + } else if (mesh->mNumFaces > std::numeric_limits::max() / sizeof(aiFace)) { + throw DeadlyImportError("AC3D: Too many faces, would run out of memory"); + } aiFace* faces = mesh->mFaces = new aiFace[mesh->mNumFaces]; mesh->mNumVertices = (*cit).second; + if (mesh->mNumVertices == 0) { + throw DeadlyImportError("AC3D: No vertices"); + } else if (mesh->mNumVertices > std::numeric_limits::max() / sizeof(aiVector3D)) { + throw DeadlyImportError("AC3D: Too many vertices, would run out of memory"); + } aiVector3D* vertices = mesh->mVertices = new aiVector3D[mesh->mNumVertices]; unsigned int cur = 0; diff --git a/code/MD3Loader.cpp b/code/MD3Loader.cpp index f7fafefdd..5e9a1a3dc 100644 --- a/code/MD3Loader.cpp +++ b/code/MD3Loader.cpp @@ -783,6 +783,11 @@ void MD3Importer::InternReadFile( const std::string& pFile, // Allocate output storage pScene->mNumMeshes = pcHeader->NUM_SURFACES; + if (pcHeader->NUM_SURFACES == 0) { + throw DeadlyImportError("MD3: No surfaces"); + } else if (pcHeader->NUM_SURFACES > std::numeric_limits::max() / sizeof(aiMesh)) { + throw DeadlyImportError("MD3: Too many surfaces, would run out of memory"); + } pScene->mMeshes = new aiMesh*[pScene->mNumMeshes]; pScene->mNumMaterials = pcHeader->NUM_SURFACES; diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index 02232fcd1..7ade10f62 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -380,6 +380,11 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, // Copy vertices of this mesh instance pMesh->mNumVertices = numIndices; + if (pMesh->mNumVertices == 0) { + throw DeadlyImportError( "OBJ: no vertices" ); + } else if (pMesh->mNumVertices > std::numeric_limits::max() / sizeof(aiVector3D)) { + throw DeadlyImportError( "OBJ: Too many vertices, would run out of memory" ); + } pMesh->mVertices = new aiVector3D[ pMesh->mNumVertices ]; // Allocate buffer for normal vectors From 0b0ba2ec4dd161bfe7a16e6d5133c7e285c5a242 Mon Sep 17 00:00:00 2001 From: Turo Lamminen Date: Thu, 13 Aug 2015 13:01:49 +0300 Subject: [PATCH 5/5] Refactor logic which checks for too large allocations It's now easier to change the limit --- code/ACLoader.cpp | 6 +++--- code/MD3Loader.cpp | 4 +++- code/ObjFileImporter.cpp | 2 +- include/assimp/defs.h | 8 ++++++++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/code/ACLoader.cpp b/code/ACLoader.cpp index 8d2fac171..8e62da442 100644 --- a/code/ACLoader.cpp +++ b/code/ACLoader.cpp @@ -296,7 +296,7 @@ void AC3DImporter::LoadObjectSection(std::vector& objects) SkipSpaces(&buffer); unsigned int t = strtoul10(buffer,&buffer); - if (t >= std::numeric_limits::max() / sizeof(aiVector3D)) { + if (t >= AI_MAX_ALLOC(aiVector3D)) { throw DeadlyImportError("AC3D: Too many vertices, would run out of memory"); } obj.vertices.reserve(t); @@ -589,7 +589,7 @@ aiNode* AC3DImporter::ConvertObjectSection(Object& object, mesh->mNumFaces = (*cit).first; if (mesh->mNumFaces == 0) { throw DeadlyImportError("AC3D: No faces"); - } else if (mesh->mNumFaces > std::numeric_limits::max() / sizeof(aiFace)) { + } else if (mesh->mNumFaces > AI_MAX_ALLOC(aiFace)) { throw DeadlyImportError("AC3D: Too many faces, would run out of memory"); } aiFace* faces = mesh->mFaces = new aiFace[mesh->mNumFaces]; @@ -597,7 +597,7 @@ aiNode* AC3DImporter::ConvertObjectSection(Object& object, mesh->mNumVertices = (*cit).second; if (mesh->mNumVertices == 0) { throw DeadlyImportError("AC3D: No vertices"); - } else if (mesh->mNumVertices > std::numeric_limits::max() / sizeof(aiVector3D)) { + } else if (mesh->mNumVertices > AI_MAX_ALLOC(aiVector3D)) { throw DeadlyImportError("AC3D: Too many vertices, would run out of memory"); } aiVector3D* vertices = mesh->mVertices = new aiVector3D[mesh->mNumVertices]; diff --git a/code/MD3Loader.cpp b/code/MD3Loader.cpp index 5e9a1a3dc..3852a64a2 100644 --- a/code/MD3Loader.cpp +++ b/code/MD3Loader.cpp @@ -785,7 +785,9 @@ void MD3Importer::InternReadFile( const std::string& pFile, pScene->mNumMeshes = pcHeader->NUM_SURFACES; if (pcHeader->NUM_SURFACES == 0) { throw DeadlyImportError("MD3: No surfaces"); - } else if (pcHeader->NUM_SURFACES > std::numeric_limits::max() / sizeof(aiMesh)) { + } else if (pcHeader->NUM_SURFACES > AI_MAX_ALLOC(aiMesh)) { + // We allocate pointers but check against the size of aiMesh + // since those pointers will eventually have to point to real objects throw DeadlyImportError("MD3: Too many surfaces, would run out of memory"); } pScene->mMeshes = new aiMesh*[pScene->mNumMeshes]; diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index 7ade10f62..e4046b53b 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -382,7 +382,7 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, pMesh->mNumVertices = numIndices; if (pMesh->mNumVertices == 0) { throw DeadlyImportError( "OBJ: no vertices" ); - } else if (pMesh->mNumVertices > std::numeric_limits::max() / sizeof(aiVector3D)) { + } else if (pMesh->mNumVertices > AI_MAX_ALLOC(aiVector3D)) { throw DeadlyImportError( "OBJ: Too many vertices, would run out of memory" ); } pMesh->mVertices = new aiVector3D[ pMesh->mNumVertices ]; diff --git a/include/assimp/defs.h b/include/assimp/defs.h index b0954920e..3129a027b 100644 --- a/include/assimp/defs.h +++ b/include/assimp/defs.h @@ -276,4 +276,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # define AI_BUILD_BIG_ENDIAN #endif + +/* To avoid running out of memory + * This can be adjusted for specific use cases + * It's NOT a total limit, just a limit for individual allocations + */ +#define AI_MAX_ALLOC(type) ((256U * 1024 * 1024) / sizeof(type)) + + #endif // !! INCLUDED_AI_DEFINES_H