From 6e39c22554d47bfd4d519227e317f536a4daf9a2 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 5 Feb 2019 22:05:52 +0100 Subject: [PATCH] Fix potential security issues. --- code/AssbinExporter.cpp | 5 +++++ code/AssimpCExport.cpp | 1 - code/AssxmlExporter.cpp | 9 ++++++-- code/ColladaParser.cpp | 2 +- code/ValidateDataStructure.cpp | 28 ++++++++++++------------- tools/assimp_cmd/WriteDumb.cpp | 16 +++++++++++++-- tools/assimp_view/MessageProc.cpp | 34 +------------------------------ 7 files changed, 41 insertions(+), 54 deletions(-) diff --git a/code/AssbinExporter.cpp b/code/AssbinExporter.cpp index 77c8d1118..03b0368ce 100644 --- a/code/AssbinExporter.cpp +++ b/code/AssbinExporter.cpp @@ -760,7 +760,12 @@ public: if (!out) return; time_t tt = time(NULL); +#if _WIN32 tm* p = gmtime(&tt); +#else + struct tm now; + tm* p = gmtime_r(&tt, &now); +#endif // header char s[64]; diff --git a/code/AssimpCExport.cpp b/code/AssimpCExport.cpp index 5121ce39c..beb3ec3c5 100644 --- a/code/AssimpCExport.cpp +++ b/code/AssimpCExport.cpp @@ -60,7 +60,6 @@ ASSIMP_API size_t aiGetExportFormatCount(void) return Exporter().GetExportFormatCount(); } - // ------------------------------------------------------------------------------------------------ ASSIMP_API const aiExportFormatDesc* aiGetExportFormatDescription( size_t index) { diff --git a/code/AssxmlExporter.cpp b/code/AssxmlExporter.cpp index fafee0e80..fc9a6bae5 100644 --- a/code/AssxmlExporter.cpp +++ b/code/AssxmlExporter.cpp @@ -184,8 +184,13 @@ static std::string encodeXML(const std::string& data) { static void WriteDump(const aiScene* scene, IOStream* io, bool shortened) { time_t tt = ::time( NULL ); - tm* p = ::gmtime( &tt ); - ai_assert( nullptr != p ); +#if _WIN32 + tm* p = gmtime(&tt); +#else + struct tm now; + tm* p = gmtime_r(&tt, &now); +#endif + ai_assert(nullptr != p); // write header std::string header( diff --git a/code/ColladaParser.cpp b/code/ColladaParser.cpp index 4d599a0c7..0fa59362b 100644 --- a/code/ColladaParser.cpp +++ b/code/ColladaParser.cpp @@ -2362,7 +2362,7 @@ size_t ColladaParser::ReadPrimitives( Mesh* pMesh, std::vector& pP if( expectedPointCount > 0 && indices.size() != expectedPointCount * numOffsets) { if (pPrimType == Prim_Lines) { // HACK: We just fix this number since SketchUp 15.3.331 writes the wrong 'count' for 'lines' - ReportWarning( "Expected different index count in

element, %d instead of %d.", indices.size(), expectedPointCount * numOffsets); + ReportWarning( "Expected different index count in

element, %zu instead of %zu.", indices.size(), expectedPointCount * numOffsets); pNumPrimitives = (indices.size() / numOffsets) / 2; } else ThrowException( "Expected different index count in

element."); diff --git a/code/ValidateDataStructure.cpp b/code/ValidateDataStructure.cpp index 7db05a4e3..405670bdd 100644 --- a/code/ValidateDataStructure.cpp +++ b/code/ValidateDataStructure.cpp @@ -180,23 +180,21 @@ inline void ValidateDSProcess::DoValidationEx(T** parray, unsigned int size, // ------------------------------------------------------------------------------------------------ template -inline void ValidateDSProcess::DoValidationWithNameCheck(T** array, - unsigned int size, const char* firstName, - const char* secondName) -{ +inline +void ValidateDSProcess::DoValidationWithNameCheck(T** array, unsigned int size, const char* firstName, const char* secondName) { // validate all entries DoValidationEx(array,size,firstName,secondName); - for (unsigned int i = 0; i < size;++i) - { + for (unsigned int i = 0; i < size;++i) { int res = HasNameMatch(array[i]->mName,mScene->mRootNode); - if (!res) { + if (0 == res) { + const std::string name = static_cast(array[i]->mName.data); ReportError("aiScene::%s[%i] has no corresponding node in the scene graph (%s)", - firstName,i,array[i]->mName.data); - } - else if (1 != res) { + firstName,i, name.c_str()); + } else if (1 != res) { + const std::string name = static_cast(array[i]->mName.data); ReportError("aiScene::%s[%i]: there are more than one nodes with %s as name", - firstName,i,array[i]->mName.data); + firstName,i, name.c_str()); } } } @@ -699,7 +697,7 @@ void ValidateDSProcess::Validate( const aiMaterial* pMaterial) if (prop->mDataLength < 5 || prop->mDataLength < 4 + (*reinterpret_cast(prop->mData)) + 1) { ReportError("aiMaterial::mProperties[%i].mDataLength is " "too small to contain a string (%i, needed: %i)", - i,prop->mDataLength,sizeof(aiString)); + i,prop->mDataLength,static_cast(sizeof(aiString))); } if(prop->mData[prop->mDataLength-1]) { ReportError("Missing null-terminator in string material property"); @@ -710,14 +708,14 @@ void ValidateDSProcess::Validate( const aiMaterial* pMaterial) if (prop->mDataLength < sizeof(float)) { ReportError("aiMaterial::mProperties[%i].mDataLength is " "too small to contain a float (%i, needed: %i)", - i,prop->mDataLength,sizeof(float)); + i,prop->mDataLength, static_cast(sizeof(float))); } } else if (aiPTI_Integer == prop->mType) { if (prop->mDataLength < sizeof(int)) { ReportError("aiMaterial::mProperties[%i].mDataLength is " "too small to contain an integer (%i, needed: %i)", - i,prop->mDataLength,sizeof(int)); + i,prop->mDataLength, static_cast(sizeof(int))); } } // TODO: check whether there is a key with an unknown name ... @@ -955,7 +953,7 @@ void ValidateDSProcess::Validate( const aiString* pString) { if (pString->length > MAXLEN) { - this->ReportError("aiString::length is too large (%i, maximum is %i)", + this->ReportError("aiString::length is too large (%i, maximum is %lu)", pString->length,MAXLEN); } const char* sz = pString->data; diff --git a/tools/assimp_cmd/WriteDumb.cpp b/tools/assimp_cmd/WriteDumb.cpp index 0bcd5321c..569749994 100644 --- a/tools/assimp_cmd/WriteDumb.cpp +++ b/tools/assimp_cmd/WriteDumb.cpp @@ -679,7 +679,13 @@ void WriteBinaryDump(const aiScene* scene, FILE* _out, const char* src, const ch shortened = _shortened; time_t tt = time(NULL); - tm* p = gmtime(&tt); +#if _WIN32 + tm* p = gmtime(&tt); +#else + struct tm now; + tm* p = gmtime_r(&tt, &now); +#endif + ai_assert(nullptr != p); // header fprintf(out,"ASSIMP.binary-dump.%s",asctime(p)); @@ -861,7 +867,13 @@ static std::string encodeXML(const std::string& data) { void WriteDump(const aiScene* scene, FILE* out, const char* src, const char* cmd, bool shortened) { time_t tt = ::time(NULL); - tm* p = ::gmtime(&tt); +#if _WIN32 + tm* p = gmtime(&tt); +#else + struct tm now; + tm* p = gmtime_r(&tt, &now); +#endif + ai_assert(nullptr != p); std::string c = cmd; std::string::size_type s; diff --git a/tools/assimp_view/MessageProc.cpp b/tools/assimp_view/MessageProc.cpp index 8afecd602..2855c1fa7 100644 --- a/tools/assimp_view/MessageProc.cpp +++ b/tools/assimp_view/MessageProc.cpp @@ -852,31 +852,6 @@ void OpenAsset() { strcpy(szCur,"*.*"); szCur[4] = 0; - /*DWORD lStructSize; - HWND hwndOwner; - HINSTANCE hInstance; - LPCWSTR lpstrFilter; - LPWSTR lpstrCustomFilter; - DWORD nMaxCustFilter; - DWORD nFilterIndex; - LPWSTR lpstrFile; - DWORD nMaxFile; - LPWSTR lpstrFileTitle; - DWORD nMaxFileTitle; - LPCWSTR lpstrInitialDir; - LPCWSTR lpstrTitle; - DWORD Flags; - WORD nFileOffset; - WORD nFileExtension; - LPCWSTR lpstrDefExt; - LPARAM lCustData; - LPOFNHOOKPROC lpfnHook; - LPCWSTR lpTemplateName; -#ifdef _MAC - LPEDITMENU lpEditInfo; - LPCSTR lpstrPrompt;*/ - - OPENFILENAME sFilename1; ZeroMemory(&sFilename1, sizeof(sFilename1)); sFilename1.lStructSize = sizeof(sFilename1); @@ -891,14 +866,7 @@ void OpenAsset() { sFilename1.nMaxFileTitle = 0; sFilename1.lpstrInitialDir = NULL; sFilename1.Flags = OFN_OVERWRITEPROMPT | OFN_HIDEREADONLY | OFN_NOCHANGEDIR; - /*OPENFILENAME sFilename1 = { - sizeof(OPENFILENAME), - g_hDlg, GetModuleHandle(NULL), szList, NULL, 0, 1, - szFileName, MAX_PATH, NULL, 0, NULL, - "Import Asset into ASSIMP", - OFN_OVERWRITEPROMPT | OFN_HIDEREADONLY | OFN_NOCHANGEDIR, - 0, 1, ".x", 0, NULL, NULL - };*/ + if (GetOpenFileName(&sFilename1) == 0) { return; }