From f4d3b6e8628c4148c636cb31f859b92f833d9a29 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 2 Jun 2023 14:36:03 +0000 Subject: [PATCH 1/4] Fix Stack-buffer-overflow READ in aiMaterial::AddBinaryProperty --- code/AssetLib/MDL/MDLMaterialLoader.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/AssetLib/MDL/MDLMaterialLoader.cpp b/code/AssetLib/MDL/MDLMaterialLoader.cpp index 799368264..aeefa5a04 100644 --- a/code/AssetLib/MDL/MDLMaterialLoader.cpp +++ b/code/AssetLib/MDL/MDLMaterialLoader.cpp @@ -493,12 +493,12 @@ void MDLImporter::ParseSkinLump_3DGS_MDL7( aiString szFile; const size_t iLen = strlen((const char *)szCurrent); - size_t iLen2 = iLen + 1; - iLen2 = iLen2 > MAXLEN ? MAXLEN : iLen2; + size_t iLen2 = iLen > MAXLEN - 1 ? MAXLEN - 1: iLen; memcpy(szFile.data, (const char *)szCurrent, iLen2); + szFile.data[iLen2] = '\0'; szFile.length = static_cast(iLen2); - szCurrent += iLen2; + szCurrent += iLen2 + 1; // place this as diffuse texture pcMatOut->AddProperty(&szFile, AI_MATKEY_TEXTURE_DIFFUSE(0)); From 57a55aa4d4adda4473d0a38066f0cbab770de75f Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 2 Jun 2023 14:36:19 +0000 Subject: [PATCH 2/4] Fix memory leaks --- code/AssetLib/HMP/HMPLoader.cpp | 16 ++++++---------- code/Material/MaterialSystem.cpp | 7 +++---- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/code/AssetLib/HMP/HMPLoader.cpp b/code/AssetLib/HMP/HMPLoader.cpp index 3dd27eb02..431783a6a 100644 --- a/code/AssetLib/HMP/HMPLoader.cpp +++ b/code/AssetLib/HMP/HMPLoader.cpp @@ -115,7 +115,9 @@ void HMPImporter::InternReadFile(const std::string &pFile, throw DeadlyImportError("HMP File is too small."); // Allocate storage and copy the contents of the file to a memory buffer - mBuffer = new uint8_t[fileSize]; + auto deleter=[this](uint8_t* ptr){ delete[] ptr; mBuffer = nullptr; }; + std::unique_ptr buffer(new uint8_t[fileSize], deleter); + mBuffer = buffer.get(); file->Read((void *)mBuffer, 1, fileSize); iFileSize = (unsigned int)fileSize; @@ -143,9 +145,6 @@ void HMPImporter::InternReadFile(const std::string &pFile, // Print the magic word to the logger std::string szBuffer = ai_str_toprintable((const char *)&iMagic, sizeof(iMagic)); - delete[] mBuffer; - mBuffer = nullptr; - // We're definitely unable to load this file throw DeadlyImportError("Unknown HMP subformat ", pFile, ". Magic word (", szBuffer, ") is not known"); @@ -153,9 +152,6 @@ void HMPImporter::InternReadFile(const std::string &pFile, // Set the AI_SCENE_FLAGS_TERRAIN bit pScene->mFlags |= AI_SCENE_FLAGS_TERRAIN; - - delete[] mBuffer; - mBuffer = nullptr; } // ------------------------------------------------------------------------------------------------ @@ -445,11 +441,11 @@ void HMPImporter::ReadFirstSkin(unsigned int iNumSkins, const unsigned char *szC szCursor += sizeof(uint32_t); // allocate an output material - aiMaterial *pcMat = new aiMaterial(); + std::unique_ptr pcMat(new aiMaterial()); // read the skin, this works exactly as for MDL7 ParseSkinLump_3DGS_MDL7(szCursor, &szCursor, - pcMat, iType, iWidth, iHeight); + pcMat.get(), iType, iWidth, iHeight); // now we need to skip any other skins ... for (unsigned int i = 1; i < iNumSkins; ++i) { @@ -468,7 +464,7 @@ void HMPImporter::ReadFirstSkin(unsigned int iNumSkins, const unsigned char *szC // setup the material ... pScene->mNumMaterials = 1; pScene->mMaterials = new aiMaterial *[1]; - pScene->mMaterials[0] = pcMat; + pScene->mMaterials[0] = pcMat.release(); *szCursorOut = szCursor; } diff --git a/code/Material/MaterialSystem.cpp b/code/Material/MaterialSystem.cpp index b2f738959..ae9d038d9 100644 --- a/code/Material/MaterialSystem.cpp +++ b/code/Material/MaterialSystem.cpp @@ -473,7 +473,7 @@ aiReturn aiMaterial::AddBinaryProperty(const void *pInput, } // Allocate a new material property - aiMaterialProperty *pcNew = new aiMaterialProperty(); + std::unique_ptr pcNew(new aiMaterialProperty()); // .. and fill it pcNew->mType = pType; @@ -489,7 +489,7 @@ aiReturn aiMaterial::AddBinaryProperty(const void *pInput, strcpy(pcNew->mKey.data, pKey); if (UINT_MAX != iOutIndex) { - mProperties[iOutIndex] = pcNew; + mProperties[iOutIndex] = pcNew.release(); return AI_SUCCESS; } @@ -502,7 +502,6 @@ aiReturn aiMaterial::AddBinaryProperty(const void *pInput, try { ppTemp = new aiMaterialProperty *[mNumAllocated]; } catch (std::bad_alloc &) { - delete pcNew; return AI_OUTOFMEMORY; } @@ -513,7 +512,7 @@ aiReturn aiMaterial::AddBinaryProperty(const void *pInput, mProperties = ppTemp; } // push back ... - mProperties[mNumProperties++] = pcNew; + mProperties[mNumProperties++] = pcNew.release(); return AI_SUCCESS; } From 6c5fe9d76f33750c37ce2675622883161cad51fb Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 2 Jun 2023 15:08:50 +0000 Subject: [PATCH 3/4] Add missing include --- code/Material/MaterialSystem.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/code/Material/MaterialSystem.cpp b/code/Material/MaterialSystem.cpp index ae9d038d9..cc8ca2f88 100644 --- a/code/Material/MaterialSystem.cpp +++ b/code/Material/MaterialSystem.cpp @@ -51,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include using namespace Assimp; From f5683b6f3a94510634276b3ab5198fb0a71ed343 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 5 Jun 2023 14:27:21 +0200 Subject: [PATCH 4/4] Update MDLMaterialLoader.cpp Add parentheses --- code/AssetLib/MDL/MDLMaterialLoader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/MDL/MDLMaterialLoader.cpp b/code/AssetLib/MDL/MDLMaterialLoader.cpp index aeefa5a04..5f5dde96a 100644 --- a/code/AssetLib/MDL/MDLMaterialLoader.cpp +++ b/code/AssetLib/MDL/MDLMaterialLoader.cpp @@ -493,7 +493,7 @@ void MDLImporter::ParseSkinLump_3DGS_MDL7( aiString szFile; const size_t iLen = strlen((const char *)szCurrent); - size_t iLen2 = iLen > MAXLEN - 1 ? MAXLEN - 1: iLen; + size_t iLen2 = iLen > (MAXLEN - 1) ? (MAXLEN - 1) : iLen; memcpy(szFile.data, (const char *)szCurrent, iLen2); szFile.data[iLen2] = '\0'; szFile.length = static_cast(iLen2);