From 4a65e76ca760cc71eda68ceb35a7d05b7c0b58e6 Mon Sep 17 00:00:00 2001 From: Faule Socke Date: Sat, 23 Dec 2017 17:34:39 +0100 Subject: [PATCH] Fix buffer overflow in obj loader The overflow-checking code in ObjFileImporter::createVertexArray is at the wrong position, allowing buffer overflows in preceding code. This fix moves the code to the right spot. An actual overflow can be caused by usign some more bugs and weird behaviours and injecting a malformed line statement into the object file, containing only one index. Such a malformed file could for example look like: o 1 v 0 0 0 v 1 1 1 v 2 2 2 l 1 f 1 2 3 Because the code in ObjFileImporter::createTopology incorrectly handles line-type faces containing only one index (in line 364), it underestimates the number of required indices and therefore causes the buffer allocated in line 421 to be too small. I believe, the correct fix for this would be in the parser and rejecting such faces early. However the overflow check was misplaced anyway. If you can't reproduce a crash, just insert some more "l 1" lines before the "f 1 2 3" line until it crashes. The behaviour of heap buffer overflows strongly depends on memory layout and allocation history. --- code/ObjFileImporter.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/code/ObjFileImporter.cpp b/code/ObjFileImporter.cpp index e1a084d62..10c463c36 100644 --- a/code/ObjFileImporter.cpp +++ b/code/ObjFileImporter.cpp @@ -448,6 +448,10 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, throw DeadlyImportError( "OBJ: vertex index out of range" ); } + if ( pMesh->mNumVertices <= newIndex ) { + throw DeadlyImportError("OBJ: bad vertex index"); + } + pMesh->mVertices[ newIndex ] = pModel->m_Vertices[ vertex ]; // Copy all normals @@ -479,10 +483,6 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel, pMesh->mTextureCoords[ 0 ][ newIndex ] = aiVector3D( coord3d.x, coord3d.y, coord3d.z ); } - if ( pMesh->mNumVertices <= newIndex ) { - throw DeadlyImportError("OBJ: bad vertex index"); - } - // Get destination face aiFace *pDestFace = &pMesh->mFaces[ outIndex ];