From 52e5c3f39ecd9ed241048b6f1bf3e682adb64194 Mon Sep 17 00:00:00 2001 From: Marco Feuerstein Date: Fri, 21 Jul 2023 09:48:45 +0200 Subject: [PATCH 1/6] Fix violation of strict aliasing rule. --- code/Common/BaseImporter.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/code/Common/BaseImporter.cpp b/code/Common/BaseImporter.cpp index d2ff4a9dd..a169c8a10 100644 --- a/code/Common/BaseImporter.cpp +++ b/code/Common/BaseImporter.cpp @@ -312,12 +312,7 @@ std::string BaseImporter::GetExtension(const std::string &pFile) { if (!pIOHandler) { return false; } - union { - const char *magic; - const uint16_t *magic_u16; - const uint32_t *magic_u32; - }; - magic = reinterpret_cast(_magic); + const char *magic = reinterpret_cast(_magic); std::unique_ptr pStream(pIOHandler->Open(pFile)); if (pStream) { @@ -339,15 +334,15 @@ std::string BaseImporter::GetExtension(const std::string &pFile) { // that's just for convenience, the chance that we cause conflicts // is quite low and it can save some lines and prevent nasty bugs if (2 == size) { - uint16_t rev = *magic_u16; - ByteSwap::Swap(&rev); - if (data_u16[0] == *magic_u16 || data_u16[0] == rev) { + uint16_t magic_u16; + memcpy(&magic_u16, magic, 2); + if (data_u16[0] == magic_u16 || data_u16[0] == ByteSwap::Swapped(magic_u16)) { return true; } } else if (4 == size) { - uint32_t rev = *magic_u32; - ByteSwap::Swap(&rev); - if (data_u32[0] == *magic_u32 || data_u32[0] == rev) { + uint32_t magic_u32; + memcpy(&magic_u32, magic, 4); + if (data_u32[0] == magic_u32 || data_u32[0] == ByteSwap::Swapped(magic_u32)) { return true; } } else { From d7dc88e0d04726d0e0e053832cae8c506fa02df5 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 1 Aug 2023 13:04:16 +0000 Subject: [PATCH 2/6] Fix UNKNOWN READ in Assimp::MDLImporter::InternReadFile_Quake1 --- code/AssetLib/MDL/MDLLoader.cpp | 10 ++++++++-- code/AssetLib/MDL/MDLLoader.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/MDL/MDLLoader.cpp b/code/AssetLib/MDL/MDLLoader.cpp index 098b53e76..9efbc821c 100644 --- a/code/AssetLib/MDL/MDLLoader.cpp +++ b/code/AssetLib/MDL/MDLLoader.cpp @@ -271,10 +271,16 @@ void MDLImporter::InternReadFile(const std::string &pFile, } } +// ------------------------------------------------------------------------------------------------ +// Check whether we're still inside the valid file range +bool MDLImporter::IsPosValid(const void *szPos) { + return szPos && (const unsigned char *)szPos <= this->mBuffer + this->iFileSize && szPos >= this->mBuffer; +} + // ------------------------------------------------------------------------------------------------ // Check whether we're still inside the valid file range void MDLImporter::SizeCheck(const void *szPos) { - if (!szPos || (const unsigned char *)szPos > this->mBuffer + this->iFileSize || szPos < this->mBuffer) { + if (!IsPosValid(szPos)) { throw DeadlyImportError("Invalid MDL file. The file is too small " "or contains invalid data."); } @@ -284,7 +290,7 @@ void MDLImporter::SizeCheck(const void *szPos) { // Just for debugging purposes void MDLImporter::SizeCheck(const void *szPos, const char *szFile, unsigned int iLine) { ai_assert(nullptr != szFile); - if (!szPos || (const unsigned char *)szPos > mBuffer + iFileSize) { + if (!IsPosValid(szPos)) { // remove a directory if there is one const char *szFilePtr = ::strrchr(szFile, '\\'); if (!szFilePtr) { diff --git a/code/AssetLib/MDL/MDLLoader.h b/code/AssetLib/MDL/MDLLoader.h index 433100938..de690b1e7 100644 --- a/code/AssetLib/MDL/MDLLoader.h +++ b/code/AssetLib/MDL/MDLLoader.h @@ -150,6 +150,7 @@ protected: */ void SizeCheck(const void* szPos); void SizeCheck(const void* szPos, const char* szFile, unsigned int iLine); + bool IsPosValid(const void* szPos); // ------------------------------------------------------------------- /** Validate the header data structure of a game studio MDL7 file From f7e7f82b9dbb63369c8d22261cbd291e57c8bb4f Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 3 Aug 2023 17:10:17 +0000 Subject: [PATCH 3/6] Add const --- code/AssetLib/MDL/MDLLoader.cpp | 2 +- code/AssetLib/MDL/MDLLoader.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/MDL/MDLLoader.cpp b/code/AssetLib/MDL/MDLLoader.cpp index 9efbc821c..7b2ec7115 100644 --- a/code/AssetLib/MDL/MDLLoader.cpp +++ b/code/AssetLib/MDL/MDLLoader.cpp @@ -273,7 +273,7 @@ void MDLImporter::InternReadFile(const std::string &pFile, // ------------------------------------------------------------------------------------------------ // Check whether we're still inside the valid file range -bool MDLImporter::IsPosValid(const void *szPos) { +bool MDLImporter::IsPosValid(const void *szPos) const { return szPos && (const unsigned char *)szPos <= this->mBuffer + this->iFileSize && szPos >= this->mBuffer; } diff --git a/code/AssetLib/MDL/MDLLoader.h b/code/AssetLib/MDL/MDLLoader.h index de690b1e7..44ff21e3e 100644 --- a/code/AssetLib/MDL/MDLLoader.h +++ b/code/AssetLib/MDL/MDLLoader.h @@ -150,7 +150,7 @@ protected: */ void SizeCheck(const void* szPos); void SizeCheck(const void* szPos, const char* szFile, unsigned int iLine); - bool IsPosValid(const void* szPos); + bool IsPosValid(const void* szPos) const; // ------------------------------------------------------------------- /** Validate the header data structure of a game studio MDL7 file From b9460dd9591583fa287cf031bf57629bffb20a84 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 8 Aug 2023 16:01:00 +0000 Subject: [PATCH 4/6] Fix UNKNOWN READ in std::__1::basic_string, std::__1::allocator #include #include +#include using namespace Assimp; @@ -160,6 +161,9 @@ void NDOImporter::InternReadFile( const std::string& pFile, temp = file_format >= 12 ? reader.GetU4() : reader.GetU2(); head = (const char*)reader.GetPtr(); + if (std::numeric_limits::max() - 76 < temp) { + throw DeadlyImportError("Invalid name length"); + } reader.IncPtr(temp + 76); /* skip unknown stuff */ obj.name = std::string(head, temp); From 2baadf2fe53718079ba86dc2b6510716a51b809e Mon Sep 17 00:00:00 2001 From: Pavel Rojtberg Date: Tue, 8 Aug 2023 18:34:13 +0200 Subject: [PATCH 5/6] Be more precise regarding index buffer --- include/assimp/postprocess.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/assimp/postprocess.h b/include/assimp/postprocess.h index cdcbf0577..d69c2924e 100644 --- a/include/assimp/postprocess.h +++ b/include/assimp/postprocess.h @@ -94,6 +94,7 @@ enum aiPostProcessSteps * indexed geometry, this step is compulsory or you'll just waste rendering * time. If this flag is not specified, no vertices are referenced by * more than one face and no index buffer is required for rendering. + * Unless #aiProcess_Triangulate is specified. Then you need one regardless. */ aiProcess_JoinIdenticalVertices = 0x2, From 20a2cc4c94e4acd29a417f76c516c211b20477d8 Mon Sep 17 00:00:00 2001 From: Pavel Rojtberg Date: Wed, 9 Aug 2023 02:05:44 +0200 Subject: [PATCH 6/6] it is the importer, not the postproc --- include/assimp/postprocess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/assimp/postprocess.h b/include/assimp/postprocess.h index d69c2924e..a905a8be0 100644 --- a/include/assimp/postprocess.h +++ b/include/assimp/postprocess.h @@ -94,7 +94,7 @@ enum aiPostProcessSteps * indexed geometry, this step is compulsory or you'll just waste rendering * time. If this flag is not specified, no vertices are referenced by * more than one face and no index buffer is required for rendering. - * Unless #aiProcess_Triangulate is specified. Then you need one regardless. + * Unless the importer (like ply) had to split vertices. Then you need one regardless. */ aiProcess_JoinIdenticalVertices = 0x2,