Fix overflow by face allocation.

code/Q3BSPFileImporter.cpp

@@ -252,14 +252,13 @@ bool Q3BSPFileImporter::findFirstMapInArchive( Q3BSPZipArchive &rArchive, std::s
 // ------------------------------------------------------------------------------------------------
 //  Creates the assimp specific data.
 void Q3BSPFileImporter::CreateDataFromImport( const Q3BSP::Q3BSPModel *pModel, aiScene* pScene,
-                                             Q3BSPZipArchive *pArchive )
-{
-    if ( NULL == pModel || NULL == pScene )
+        Q3BSPZipArchive *pArchive ) {
+    if (nullptr == pModel || nullptr == pScene) {
         return;
+    }
 
     pScene->mRootNode = new aiNode;
-    if ( !pModel->m_ModelName.empty() )
-    {
+    if ( !pModel->m_ModelName.empty() ) {
         pScene->mRootNode->mName.Set( pModel->m_ModelName );
     }
 
@@ -276,32 +275,24 @@ void Q3BSPFileImporter::CreateDataFromImport( const Q3BSP::Q3BSPModel *pModel, a
 // ------------------------------------------------------------------------------------------------
 //  Creates all assimp nodes.
 void Q3BSPFileImporter::CreateNodes( const Q3BSP::Q3BSPModel *pModel, aiScene* pScene,
-                                    aiNode *pParent )
-{
-    ai_assert( NULL != pModel );
-    if ( NULL == pModel )
-    {
+        aiNode *pParent ) {
+    if ( nullptr == pModel ) {
         return;
     }
 
-    unsigned int matIdx = 0;
+    unsigned int matIdx( 0 );
     std::vector<aiMesh*> MeshArray;
     std::vector<aiNode*> NodeArray;
-    for ( FaceMapIt it = m_MaterialLookupMap.begin(); it != m_MaterialLookupMap.end(); ++it )
-    {
+    for ( FaceMapIt it = m_MaterialLookupMap.begin(); it != m_MaterialLookupMap.end(); ++it ) {
         std::vector<Q3BSP::sQ3BSPFace*> *pArray = (*it).second;
         size_t numVerts = countData( *pArray );
-        if ( 0 != numVerts )
-        {
+        if ( 0 != numVerts ) {
             aiMesh* pMesh = new aiMesh;
             aiNode *pNode = CreateTopology( pModel, matIdx, *pArray, pMesh );
-            if ( NULL != pNode )
-            {
+            if ( nullptr != pNode ) {
                 NodeArray.push_back( pNode );
                 MeshArray.push_back( pMesh );
-            }
-            else
-            {
+            } else {
                 delete pMesh;
             }
         }
@@ -419,6 +410,14 @@ void Q3BSPFileImporter::createTriangleTopology( const Q3BSP::Q3BSPModel *pModel,
         if ( nullptr == pVertex ) {
             continue;
         }
+        if (idx > 2) {
+            idx = 0;
+            m_pCurrentFace = getNextFace(pMesh, faceIdx);
+            if (nullptr != m_pCurrentFace) {
+                m_pCurrentFace->mNumIndices = 3;
+                m_pCurrentFace->mIndices = new unsigned int[3];
+            }
+        }
 
         pMesh->mVertices[ vertIdx ].Set( pVertex->vPosition.x, pVertex->vPosition.y, pVertex->vPosition.z );
         pMesh->mNormals[ vertIdx ].Set( pVertex->vNormal.x, pVertex->vNormal.y, pVertex->vNormal.z );
@@ -430,14 +429,6 @@ void Q3BSPFileImporter::createTriangleTopology( const Q3BSP::Q3BSPModel *pModel,
         vertIdx++;
 
         idx++;
-        if ( idx > 2 ) {
-            idx = 0;
-            m_pCurrentFace = getNextFace( pMesh, faceIdx );
-            if ( nullptr != m_pCurrentFace ) {
-                m_pCurrentFace->mNumIndices = 3;
-                m_pCurrentFace->mIndices = new unsigned int[ 3 ];
-            }
-        }
     }
     //faceIdx--;
 }

code/Q3BSPFileParser.cpp

@@ -55,48 +55,44 @@ namespace Assimp {
 using namespace Q3BSP;
 
 // ------------------------------------------------------------------------------------------------
-Q3BSPFileParser::Q3BSPFileParser( const std::string &rMapName, Q3BSPZipArchive *pZipArchive ) :
+Q3BSPFileParser::Q3BSPFileParser( const std::string &mapName, Q3BSPZipArchive *pZipArchive ) :
     m_sOffset( 0 ),
     m_Data(),
-    m_pModel( NULL ),
+    m_pModel(nullptr),
     m_pZipArchive( pZipArchive )
 {
-    ai_assert( NULL != m_pZipArchive );
-    ai_assert( !rMapName.empty() );
+    ai_assert(nullptr != m_pZipArchive );
+    ai_assert( !mapName.empty() );
 
-    if ( !readData( rMapName ) )
+    if ( !readData( mapName ) )
         return;
 
     m_pModel = new Q3BSPModel;
-    m_pModel->m_ModelName = rMapName;
-    if ( !parseFile() )
-    {
+    m_pModel->m_ModelName = mapName;
+    if ( !parseFile() ) {
         delete m_pModel;
-        m_pModel = NULL;
+        m_pModel = nullptr;
     }
 }
 
 // ------------------------------------------------------------------------------------------------
-Q3BSPFileParser::~Q3BSPFileParser()
-{
+Q3BSPFileParser::~Q3BSPFileParser() {
     delete m_pModel;
-    m_pModel = NULL;
+    m_pModel = nullptr;
 }
 
 // ------------------------------------------------------------------------------------------------
-Q3BSP::Q3BSPModel *Q3BSPFileParser::getModel() const
-{
+Q3BSP::Q3BSPModel *Q3BSPFileParser::getModel() const {
     return m_pModel;
 }
 
 // ------------------------------------------------------------------------------------------------
-bool Q3BSPFileParser::readData( const std::string &rMapName )
-{
+bool Q3BSPFileParser::readData( const std::string &rMapName ) {
     if ( !m_pZipArchive->Exists( rMapName.c_str() ) )
         return false;
 
     IOStream *pMapFile = m_pZipArchive->Open( rMapName.c_str() );
-    if ( NULL == pMapFile )
+    if ( nullptr == pMapFile )
         return false;
 
     const size_t size = pMapFile->FileSize();
@@ -113,10 +109,8 @@ bool Q3BSPFileParser::readData( const std::string &rMapName )
 }
 
 // ------------------------------------------------------------------------------------------------
-bool Q3BSPFileParser::parseFile()
-{
-    if ( m_Data.empty() )
-    {
+bool Q3BSPFileParser::parseFile() {
+    if ( m_Data.empty() ) {
         return false;
     }
 
@@ -128,7 +122,7 @@ bool Q3BSPFileParser::parseFile()
     // Imports the dictionary of the level
     getLumps();
 
-    // Conunt data and prepare model data
+    // Count data and prepare model data
     countLumps();
 
     // Read in Vertices
@@ -208,7 +202,7 @@ void Q3BSPFileParser::getVertices()
 // ------------------------------------------------------------------------------------------------
 void Q3BSPFileParser::getIndices()
 {
-    ai_assert( NULL != m_pModel );
+    ai_assert(nullptr != m_pModel );
 
     sQ3BSPLump *lump = m_pModel->m_Lumps[ kMeshVerts ];
     size_t Offset = (size_t) lump->iOffset;
@@ -220,7 +214,7 @@ void Q3BSPFileParser::getIndices()
 // ------------------------------------------------------------------------------------------------
 void Q3BSPFileParser::getFaces()
 {
-    ai_assert( NULL != m_pModel );
+    ai_assert(nullptr != m_pModel );
 
     size_t Offset = m_pModel->m_Lumps[ kFaces ]->iOffset;
     for ( size_t idx = 0; idx < m_pModel->m_Faces.size(); idx++ )
@@ -235,7 +229,7 @@ void Q3BSPFileParser::getFaces()
 // ------------------------------------------------------------------------------------------------
 void Q3BSPFileParser::getTextures()
 {
-    ai_assert( NULL != m_pModel );
+    ai_assert(nullptr != m_pModel );
 
     size_t Offset = m_pModel->m_Lumps[ kTextures ]->iOffset;
     for ( size_t idx=0; idx < m_pModel->m_Textures.size(); idx++ )
@@ -250,7 +244,7 @@ void Q3BSPFileParser::getTextures()
 // ------------------------------------------------------------------------------------------------
 void Q3BSPFileParser::getLightMaps()
 {
-    ai_assert( NULL != m_pModel );
+    ai_assert(nullptr != m_pModel );
 
     size_t Offset = m_pModel->m_Lumps[kLightmaps]->iOffset;
     for ( size_t idx=0; idx < m_pModel->m_Lightmaps.size(); idx++ )
@@ -263,12 +257,10 @@ void Q3BSPFileParser::getLightMaps()
 }
 
 // ------------------------------------------------------------------------------------------------
-void Q3BSPFileParser::getEntities()
-{
-    int size = m_pModel->m_Lumps[ kEntities ]->iSize;
+void Q3BSPFileParser::getEntities() {
+    const int size = m_pModel->m_Lumps[ kEntities ]->iSize;
     m_pModel->m_EntityData.resize( size );
-    if ( size > 0 )
-    {
+    if ( size > 0 ) {
         size_t Offset = m_pModel->m_Lumps[ kEntities ]->iOffset;
         memcpy( &m_pModel->m_EntityData[ 0 ], &m_Data[ Offset ], sizeof( char ) * size );
     }

code/Q3BSPZipArchive.cpp

@@ -183,7 +183,7 @@ Q3BSPZipArchive::Q3BSPZipArchive(IOSystem* pIOHandler, const std::string& rFile)
 
         m_ZipFileHandle = unzOpen2(rFile.c_str(), &mapping);
 
-        if(m_ZipFileHandle != NULL) {
+        if(m_ZipFileHandle != nullptr) {
             mapArchive();
         }
     }
@@ -197,26 +197,23 @@ Q3BSPZipArchive::~Q3BSPZipArchive() {
     }
     m_ArchiveMap.clear();
 
-    if(m_ZipFileHandle != NULL) {
+    if(m_ZipFileHandle != nullptr) {
         unzClose(m_ZipFileHandle);
-        m_ZipFileHandle = NULL;
+        m_ZipFileHandle = nullptr;
     }
 }
 
 // ------------------------------------------------------------------------------------------------
 //  Returns true, if the archive is already open.
 bool Q3BSPZipArchive::isOpen() const {
-    return (m_ZipFileHandle != NULL);
+    return (m_ZipFileHandle != nullptr);
 }
 
 // ------------------------------------------------------------------------------------------------
 //  Returns true, if the filename is part of the archive.
 bool Q3BSPZipArchive::Exists(const char* pFile) const {
-    ai_assert(pFile != NULL);
-
     bool exist = false;
-
-    if (pFile != NULL) {
+    if (pFile != nullptr) {
         std::string rFile(pFile);
         std::map<std::string, ZipFile*>::const_iterator it = m_ArchiveMap.find(rFile);
 
@@ -241,9 +238,9 @@ char Q3BSPZipArchive::getOsSeparator() const {
 // ------------------------------------------------------------------------------------------------
 //  Opens a file, which is part of the archive.
 IOStream *Q3BSPZipArchive::Open(const char* pFile, const char* /*pMode*/) {
-    ai_assert(pFile != NULL);
+    ai_assert(pFile != nullptr);
 
-    IOStream* result = NULL;
+    IOStream* result = nullptr;
 
     std::map<std::string, ZipFile*>::iterator it = m_ArchiveMap.find(pFile);
 
@@ -258,7 +255,7 @@ IOStream *Q3BSPZipArchive::Open(const char* pFile, const char* /*pMode*/) {
 //  Close a filestream.
 void Q3BSPZipArchive::Close(IOStream *pFile) {
     (void)(pFile);
-    ai_assert(pFile != NULL);
+    ai_assert(pFile != nullptr);
 
     // We don't do anything in case the file would be opened again in the future
 }
@@ -277,7 +274,7 @@ void Q3BSPZipArchive::getFileList(std::vector<std::string> &rFileList) {
 bool Q3BSPZipArchive::mapArchive() {
     bool success = false;
 
-    if(m_ZipFileHandle != NULL) {
+    if(m_ZipFileHandle != nullptr) {
         if(m_ArchiveMap.empty()) {
             //  At first ensure file is already open
             if(unzGoToFirstFile(m_ZipFileHandle) == UNZ_OK) {