Merge pull request #1666 from aavenel/fix_obj_crash

Fix some crashes when parsing ill-formed OBJ file

code/ObjFileImporter.cpp

@@ -474,7 +474,6 @@ void ObjFileImporter::createVertexArray(const ObjFile::Model* pModel,
             if ( !pModel->m_TextureCoord.empty() && vertexIndex < pSourceFace->m_texturCoords.size())
             {
                 const unsigned int tex = pSourceFace->m_texturCoords.at( vertexIndex );
-                ai_assert( tex < pModel->m_TextureCoord.size() );
 
                 if ( tex >= pModel->m_TextureCoord.size() )
                     throw DeadlyImportError("OBJ: texture coordinate index out of range");

code/ObjFileParser.cpp

@@ -60,7 +60,7 @@ const std::string ObjFileParser::DEFAULT_MATERIAL = AI_DEFAULT_MATERIAL_NAME;
 ObjFileParser::ObjFileParser()
 : m_DataIt()
 , m_DataItEnd()
-, m_pModel( NULL )
+, m_pModel( nullptr )
 , m_uiLine( 0 )
 , m_pIO( nullptr )
 , m_progress( nullptr )
@@ -73,7 +73,7 @@ ObjFileParser::ObjFileParser( IOStreamBuffer<char> &streamBuffer, const std::str
                               const std::string &originalObjFileName) :
     m_DataIt(),
     m_DataItEnd(),
-    m_pModel(NULL),
+    m_pModel(nullptr),
     m_uiLine(0),
     m_pIO( io ),
     m_progress(progress),
@@ -82,7 +82,7 @@ ObjFileParser::ObjFileParser( IOStreamBuffer<char> &streamBuffer, const std::str
     std::fill_n(m_buffer,Buffersize,0);
 
     // Create the model instance to store all the data
-    m_pModel = new ObjFile::Model();
+    m_pModel.reset(new ObjFile::Model());
     m_pModel->m_ModelName = modelName;
 
     // create default material and store it
@@ -96,8 +96,6 @@ ObjFileParser::ObjFileParser( IOStreamBuffer<char> &streamBuffer, const std::str
 }
 
 ObjFileParser::~ObjFileParser() {
-    delete m_pModel;
-    m_pModel = NULL;
 }
 
 void ObjFileParser::setBuffer( std::vector<char> &buffer ) {
@@ -106,7 +104,7 @@ void ObjFileParser::setBuffer( std::vector<char> &buffer ) {
 }
 
 ObjFile::Model *ObjFileParser::GetModel() const {
-    return m_pModel;
+    return m_pModel.get();
 }
 
 void ObjFileParser::parseFile( IOStreamBuffer<char> &streamBuffer ) {
@@ -353,7 +351,8 @@ void ObjFileParser::getHomogeneousVector3( std::vector<aiVector3D> &point3d_arra
     copyNextWord( m_buffer, Buffersize );
     w = ( ai_real ) fast_atof( m_buffer );
 
-    ai_assert( w != 0 );
+    if (w == 0)
+      throw DeadlyImportError("OBJ: Invalid component in homogeneous vector (Division by zero)");
 
     point3d_array.push_back( aiVector3D( x/w, y/w, z/w ) );
     m_DataIt = skipLine<DataArrayIt>( m_DataIt, m_DataItEnd, m_uiLine );
@@ -478,8 +477,6 @@ void ObjFileParser::getFace( aiPrimitiveType type ) {
             } else {
                 //On error, std::atoi will return 0 which is not a valid value
                 delete face;
-                delete m_pModel;
-                m_pModel = nullptr;
                 throw DeadlyImportError("OBJ: Invalid face indice");
             }
 
@@ -641,7 +638,7 @@ void ObjFileParser::getMaterialLib() {
     m_pIO->Close( pFile );
 
     // Importing the material library
-    ObjFileMtlImporter mtlImporter( buffer, strMatName, m_pModel );
+    ObjFileMtlImporter mtlImporter( buffer, strMatName, m_pModel.get() );
 }
 
 // -------------------------------------------------------------------

code/ObjFileParser.h

@@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <vector>
 #include <string>
 #include <map>
+#include <memory>
 #include <assimp/vector2.h>
 #include <assimp/vector3.h>
 #include <assimp/mesh.h>
@@ -145,7 +146,7 @@ private:
     //! Iterator to end position of buffer
     DataArrayIt m_DataItEnd;
     //! Pointer to model instance
-    ObjFile::Model *m_pModel;
+    std::unique_ptr<ObjFile::Model> m_pModel;
     //! Current line (for debugging)
     unsigned int m_uiLine;
     //! Helper buffer

test/unit/utObjImportExport.cpp

@@ -330,6 +330,18 @@ TEST_F(utObjImportExport, homogeneous_coordinates_Test) {
     EXPECT_EQ(vertice.z, 0.8f);
 }
 
+TEST_F(utObjImportExport, homogeneous_coordinates_divide_by_zero_Test) {
+  static const std::string ObjModel =
+    "v -0.500000 0.000000 0.400000 0.\n"
+    "v -0.500000 0.000000 -0.800000 1.00000\n"
+    "v 0.500000 1.000000 -0.800000 0.5000\n"
+    "f 1 2 3\nB";
+
+  Assimp::Importer myimporter;
+  const aiScene *scene = myimporter.ReadFileFromMemory(ObjModel.c_str(), ObjModel.size(), aiProcess_ValidateDataStructure);
+  EXPECT_EQ(nullptr, scene);
+}
+
 TEST_F(utObjImportExport, 0based_array_Test) {
     static const std::string ObjModel =
         "v -0.500000 0.000000 0.400000\n"