From 6b9c47776394d9d92c3ab1db35e0a53f62d377ba Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Fri, 2 Oct 2020 13:58:55 +0100 Subject: [PATCH 1/6] The floar parsing routines are now DeadlyErrors. --- code/AssetLib/Collada/ColladaExporter.cpp | 2 +- include/assimp/fast_atof.h | 28 ++++++++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/code/AssetLib/Collada/ColladaExporter.cpp b/code/AssetLib/Collada/ColladaExporter.cpp index 567f7c8e7..251cbee3e 100644 --- a/code/AssetLib/Collada/ColladaExporter.cpp +++ b/code/AssetLib/Collada/ColladaExporter.cpp @@ -573,7 +573,7 @@ bool ColladaExporter::ReadMaterialSurface(Surface &poSurface, const aiMaterial & index_str = index_str.substr(1, std::string::npos); try { - index = (unsigned int)strtoul10_64(index_str.c_str()); + index = (unsigned int)strtoul10_64(index_str.c_str()); } catch (std::exception &error) { throw DeadlyExportError(error.what()); } diff --git a/include/assimp/fast_atof.h b/include/assimp/fast_atof.h index 6e9a1bba7..79740a1e8 100644 --- a/include/assimp/fast_atof.h +++ b/include/assimp/fast_atof.h @@ -29,6 +29,7 @@ #include "StringComparison.h" #include +#include #ifdef _MSC_VER # include @@ -185,13 +186,14 @@ unsigned int strtoul_cppstyle( const char* in, const char** out=0) { // Special version of the function, providing higher accuracy and safety // It is mainly used by fast_atof to prevent ugly and unwanted integer overflows. // ------------------------------------------------------------------------------------ +template inline uint64_t strtoul10_64( const char* in, const char** out=0, unsigned int* max_inout=0) { unsigned int cur = 0; uint64_t value = 0; if ( *in < '0' || *in > '9' ) { - throw std::invalid_argument( std::string( "The string \"" ) + in + "\" cannot be converted into a value." ); + throw DeadlyImportError("The string \"", in, "\" cannot be converted into a value." ); } for ( ;; ) { @@ -237,6 +239,7 @@ uint64_t strtoul10_64( const char* in, const char** out=0, unsigned int* max_ino // ------------------------------------------------------------------------------------ // signed variant of strtoul10_64 // ------------------------------------------------------------------------------------ +template inline int64_t strtol10_64(const char* in, const char** out = 0, unsigned int* max_inout = 0) { bool inv = (*in == '-'); @@ -244,7 +247,7 @@ int64_t strtol10_64(const char* in, const char** out = 0, unsigned int* max_inou ++in; } - int64_t value = strtoul10_64(in, out, max_inout); + int64_t value = strtoul10_64(in, out, max_inout); if (inv) { value = -value; } @@ -259,7 +262,7 @@ int64_t strtol10_64(const char* in, const char** out = 0, unsigned int* max_inou //! about 6 times faster than atof in win32. // If you find any bugs, please send them to me, niko (at) irrlicht3d.org. // ------------------------------------------------------------------------------------ -template +template inline const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) { Real f = 0; @@ -289,13 +292,13 @@ const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) if (!(c[0] >= '0' && c[0] <= '9') && !((c[0] == '.' || (check_comma && c[0] == ',')) && c[1] >= '0' && c[1] <= '9')) { - throw std::invalid_argument("Cannot parse string " - "as real number: does not start with digit " + throw ExceptionType("Cannot parse string \"", c, + "\" as a real number: does not start with digit " "or decimal point followed by digit."); } if (*c != '.' && (! check_comma || c[0] != ',')) { - f = static_cast( strtoul10_64 ( c, &c) ); + f = static_cast( strtoul10_64 ( c, &c) ); } if ((*c == '.' || (check_comma && c[0] == ',')) && c[1] >= '0' && c[1] <= '9') { @@ -310,7 +313,7 @@ const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) // number of digits to be read. AI_FAST_ATOF_RELAVANT_DECIMALS can be a value between // 1 and 15. unsigned int diff = AI_FAST_ATOF_RELAVANT_DECIMALS; - double pl = static_cast( strtoul10_64 ( c, &c, &diff )); + double pl = static_cast( strtoul10_64 ( c, &c, &diff )); pl *= fast_atof_table[diff]; f += static_cast( pl ); @@ -332,7 +335,7 @@ const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) // The reason float constants are used here is that we've seen cases where compilers // would perform such casts on compile-time constants at runtime, which would be // bad considering how frequently fast_atoreal_move is called in Assimp. - Real exp = static_cast( strtoul10_64(c, &c) ); + Real exp = static_cast( strtoul10_64(c, &c) ); if (einv) { exp = -exp; } @@ -348,26 +351,29 @@ const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) // ------------------------------------------------------------------------------------ // The same but more human. +template inline ai_real fast_atof(const char* c) { ai_real ret(0.0); - fast_atoreal_move(c, ret); + fast_atoreal_move(c, ret); return ret; } +template inline ai_real fast_atof( const char* c, const char** cout) { ai_real ret(0.0); - *cout = fast_atoreal_move(c, ret); + *cout = fast_atoreal_move(c, ret); return ret; } +template inline ai_real fast_atof( const char** inout) { ai_real ret(0.0); - *inout = fast_atoreal_move(*inout, ret); + *inout = fast_atoreal_move(*inout, ret); return ret; } From 17702605cfd4ff697e7bbf2ba50ad87db451de77 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Fri, 2 Oct 2020 14:41:36 +0100 Subject: [PATCH 2/6] Limit the number of characters printed. --- include/assimp/fast_atof.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/assimp/fast_atof.h b/include/assimp/fast_atof.h index 79740a1e8..a27863760 100644 --- a/include/assimp/fast_atof.h +++ b/include/assimp/fast_atof.h @@ -193,7 +193,8 @@ uint64_t strtoul10_64( const char* in, const char** out=0, unsigned int* max_ino uint64_t value = 0; if ( *in < '0' || *in > '9' ) { - throw DeadlyImportError("The string \"", in, "\" cannot be converted into a value." ); + // The string is known to be bad, so don't risk printing the whole thing. + throw ExceptionType("The string \"", std::substr(in, 0, 100), "\" cannot be converted into a value." ); } for ( ;; ) { @@ -292,7 +293,8 @@ const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) if (!(c[0] >= '0' && c[0] <= '9') && !((c[0] == '.' || (check_comma && c[0] == ',')) && c[1] >= '0' && c[1] <= '9')) { - throw ExceptionType("Cannot parse string \"", c, + // The string is known to be bad, so don't risk printing the whole thing. + throw ExceptionType("Cannot parse string \"", std::substr(c, 0, 100), "\" as a real number: does not start with digit " "or decimal point followed by digit."); } From 57756750f66196e9dfba680d3ea51dcc711c4b29 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Fri, 2 Oct 2020 15:20:50 +0100 Subject: [PATCH 3/6] Limit the output --- include/assimp/fast_atof.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/assimp/fast_atof.h b/include/assimp/fast_atof.h index a27863760..9ea49c85c 100644 --- a/include/assimp/fast_atof.h +++ b/include/assimp/fast_atof.h @@ -24,7 +24,6 @@ #include #include #include -#include #include #include "StringComparison.h" @@ -194,7 +193,7 @@ uint64_t strtoul10_64( const char* in, const char** out=0, unsigned int* max_ino if ( *in < '0' || *in > '9' ) { // The string is known to be bad, so don't risk printing the whole thing. - throw ExceptionType("The string \"", std::substr(in, 0, 100), "\" cannot be converted into a value." ); + throw ExceptionType("The string \"", std::string(in).substr(0, 100), "\" cannot be converted into a value." ); } for ( ;; ) { @@ -294,7 +293,7 @@ const char* fast_atoreal_move(const char* c, Real& out, bool check_comma = true) if (!(c[0] >= '0' && c[0] <= '9') && !((c[0] == '.' || (check_comma && c[0] == ',')) && c[1] >= '0' && c[1] <= '9')) { // The string is known to be bad, so don't risk printing the whole thing. - throw ExceptionType("Cannot parse string \"", std::substr(c, 0, 100), + throw ExceptionType("Cannot parse string \"", std::string(c).substr(0, 100), "\" as a real number: does not start with digit " "or decimal point followed by digit."); } From 585fb89154da1dd79cedaa1ba0a2342571a7b0f4 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Fri, 2 Oct 2020 15:25:16 +0100 Subject: [PATCH 4/6] Make an assert a DeadlyImportError. --- code/AssetLib/FBX/FBXImporter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/code/AssetLib/FBX/FBXImporter.cpp b/code/AssetLib/FBX/FBXImporter.cpp index f80b65cd4..fc3b85ef5 100644 --- a/code/AssetLib/FBX/FBXImporter.cpp +++ b/code/AssetLib/FBX/FBXImporter.cpp @@ -187,6 +187,10 @@ void FBXImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy // size relative to cm float size_relative_to_cm = doc.GlobalSettings().UnitScaleFactor(); + if (size_relative_to_cm == 0.0) + { + ThrowException("The UnitScaleFactor must be non-zero"); + } // Set FBX file scale is relative to CM must be converted to M for // assimp universal format (M) From 4bdaf20b708337f21317822cb5fb3b5dac4603a2 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Mon, 5 Oct 2020 14:23:42 +0100 Subject: [PATCH 5/6] Add comment. --- code/AssetLib/FBX/FBXImporter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/code/AssetLib/FBX/FBXImporter.cpp b/code/AssetLib/FBX/FBXImporter.cpp index fc3b85ef5..4c473510e 100644 --- a/code/AssetLib/FBX/FBXImporter.cpp +++ b/code/AssetLib/FBX/FBXImporter.cpp @@ -189,6 +189,7 @@ void FBXImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy float size_relative_to_cm = doc.GlobalSettings().UnitScaleFactor(); if (size_relative_to_cm == 0.0) { + // BaseImporter later asserts that fileScale is non-zero. ThrowException("The UnitScaleFactor must be non-zero"); } From 4006bb71f4b37cbaaddb02bae95de6607a7cee15 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Fri, 23 Oct 2020 12:01:43 +0100 Subject: [PATCH 6/6] Fixes for crashes in GLTF2 Importer --- code/AssetLib/glTF2/glTF2Asset.inl | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index 67f33d74a..badf60f5c 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -554,6 +554,16 @@ inline void BufferView::Read(Value &obj, Asset &r) { byteOffset = MemberOrDefault(obj, "byteOffset", size_t(0)); byteLength = MemberOrDefault(obj, "byteLength", size_t(0)); byteStride = MemberOrDefault(obj, "byteStride", 0u); + + // Check length + if ((byteOffset + byteLength) > buffer->byteLength) { + const uint8_t val_size = 64; + + char val[val_size]; + + ai_snprintf(val, val_size, "%llu, %llu", (unsigned long long)byteOffset, (unsigned long long)byteLength); + throw DeadlyImportError("GLTF: Buffer view with offset/length (", val, ") is out of range."); + } } inline uint8_t *BufferView::GetPointer(size_t accOffset) { @@ -627,6 +637,17 @@ inline void Accessor::Read(Value &obj, Asset &r) { const char *typestr; type = ReadMember(obj, "type", typestr) ? AttribType::FromString(typestr) : AttribType::SCALAR; + // Check length + unsigned long long byteLength = (unsigned long long)GetBytesPerComponent() * (unsigned long long)count; + if ((byteOffset + byteLength) > bufferView->byteLength || (bufferView->byteOffset + byteOffset + byteLength) > bufferView->buffer->byteLength) { + const uint8_t val_size = 64; + + char val[val_size]; + + ai_snprintf(val, val_size, "%llu, %llu", (unsigned long long)byteOffset, (unsigned long long)byteLength); + throw DeadlyImportError("GLTF: Accessor with offset/length (", val, ") is out of range."); + } + if (Value *sparseValue = FindObject(obj, "sparse")) { sparse.reset(new Sparse); // count @@ -1115,7 +1136,10 @@ inline void Mesh::Read(Value &pJSON_Object, Asset &pAsset_Root) { Mesh::AccessorList *vec = 0; if (GetAttribVector(prim, attr, vec, undPos)) { size_t idx = (attr[undPos] == '_') ? atoi(attr + undPos + 1) : 0; - if ((*vec).size() <= idx) (*vec).resize(idx + 1); + if ((*vec).size() != idx) { + throw DeadlyImportError("GLTF: Invalid attribute: ", attr, ". All indices for indexed attribute semantics must start with 0 and be continuous positive integers: TEXCOORD_0, TEXCOORD_1, etc."); + } + (*vec).resize(idx + 1); (*vec)[idx] = pAsset_Root.accessors.Retrieve(it->value.GetUint()); } }