From 3d4b54f8fcdd26308f20595ae7928a2d0470d771 Mon Sep 17 00:00:00 2001 From: rickomax Date: Wed, 19 Jul 2017 19:00:53 -0300 Subject: [PATCH 1/2] Fixed FBX 7500 Binary reading Seems that all FBX 7.5 Binary files uses 32 bits adresses. The code now is taking this in consideration. This commit fixes the https://github.com/assimp/assimp/issues/838 issue. --- code/FBXBinaryTokenizer.cpp | 113 ++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 58 deletions(-) diff --git a/code/FBXBinaryTokenizer.cpp b/code/FBXBinaryTokenizer.cpp index a02458fa5..fb57b2f97 100644 --- a/code/FBXBinaryTokenizer.cpp +++ b/code/FBXBinaryTokenizer.cpp @@ -56,47 +56,46 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace Assimp { namespace FBX { -enum Flag -{ - e_unknown_0 = 1 << 0, - e_unknown_1 = 1 << 1, - e_unknown_2 = 1 << 2, - e_unknown_3 = 1 << 3, - e_unknown_4 = 1 << 4, - e_unknown_5 = 1 << 5, - e_unknown_6 = 1 << 6, - e_unknown_7 = 1 << 7, - e_unknown_8 = 1 << 8, - e_unknown_9 = 1 << 9, - e_unknown_10 = 1 << 10, - e_unknown_11 = 1 << 11, - e_unknown_12 = 1 << 12, - e_unknown_13 = 1 << 13, - e_unknown_14 = 1 << 14, - e_unknown_15 = 1 << 15, - e_unknown_16 = 1 << 16, - e_unknown_17 = 1 << 17, - e_unknown_18 = 1 << 18, - e_unknown_19 = 1 << 19, - e_unknown_20 = 1 << 20, - e_unknown_21 = 1 << 21, - e_unknown_22 = 1 << 22, - e_unknown_23 = 1 << 23, - e_flag_field_size_64_bit = 1 << 24, // Not sure what is - e_unknown_25 = 1 << 25, - e_unknown_26 = 1 << 26, - e_unknown_27 = 1 << 27, - e_unknown_28 = 1 << 28, - e_unknown_29 = 1 << 29, - e_unknown_30 = 1 << 30, - e_unknown_31 = 1 << 31 -}; - -bool check_flag(uint32_t flags, Flag to_check) -{ - return (flags & to_check) != 0; -} - +//enum Flag +//{ +// e_unknown_0 = 1 << 0, +// e_unknown_1 = 1 << 1, +// e_unknown_2 = 1 << 2, +// e_unknown_3 = 1 << 3, +// e_unknown_4 = 1 << 4, +// e_unknown_5 = 1 << 5, +// e_unknown_6 = 1 << 6, +// e_unknown_7 = 1 << 7, +// e_unknown_8 = 1 << 8, +// e_unknown_9 = 1 << 9, +// e_unknown_10 = 1 << 10, +// e_unknown_11 = 1 << 11, +// e_unknown_12 = 1 << 12, +// e_unknown_13 = 1 << 13, +// e_unknown_14 = 1 << 14, +// e_unknown_15 = 1 << 15, +// e_unknown_16 = 1 << 16, +// e_unknown_17 = 1 << 17, +// e_unknown_18 = 1 << 18, +// e_unknown_19 = 1 << 19, +// e_unknown_20 = 1 << 20, +// e_unknown_21 = 1 << 21, +// e_unknown_22 = 1 << 22, +// e_unknown_23 = 1 << 23, +// e_flag_field_size_64_bit = 1 << 24, // Not sure what is +// e_unknown_25 = 1 << 25, +// e_unknown_26 = 1 << 26, +// e_unknown_27 = 1 << 27, +// e_unknown_28 = 1 << 28, +// e_unknown_29 = 1 << 29, +// e_unknown_30 = 1 << 30, +// e_unknown_31 = 1 << 31 +//}; +// +//bool check_flag(uint32_t flags, Flag to_check) +//{ +// return (flags & to_check) != 0; +//} // ------------------------------------------------------------------------------------------------ Token::Token(const char* sbegin, const char* send, TokenType type, unsigned int offset) : @@ -341,10 +340,11 @@ void ReadData(const char*& sbegin_out, const char*& send_out, const char* input, // ------------------------------------------------------------------------------------------------ -bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, const char* end, uint32_t const flags) +//TODO: Test FBX Binary files newer than the 7500 version to check if the 64 bits address behaviour is consistent +bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, const char* end, uint32_t const version) { // the first word contains the offset at which this block ends - const uint64_t end_offset = /*check_flag(flags, e_flag_field_size_64_bit) ? ReadDoubleWord(input, cursor, end) : */ReadWord(input, cursor, end); + const uint64_t end_offset = version == 7500 ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); // we may get 0 if reading reached the end of the file - // fbx files have a mysterious extra footer which I don't know @@ -362,10 +362,10 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, } // the second data word contains the number of properties in the scope - const uint64_t prop_count = /*check_flag(flags, e_flag_field_size_64_bit) ? ReadDoubleWord(input, cursor, end) : */ReadWord(input, cursor, end); + const uint64_t prop_count = version == 7500 ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); // the third data word contains the length of the property list - const uint64_t prop_length = /*check_flag(flags, e_flag_field_size_64_bit) ? ReadDoubleWord(input, cursor, end) :*/ ReadWord(input, cursor, end); + const uint64_t prop_length = version == 7500 ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); // now comes the name of the scope/key const char* sbeg, *send; @@ -392,7 +392,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, // at the end of each nested block, there is a NUL record to indicate // that the sub-scope exists (i.e. to distinguish between P: and P : {}) // this NUL record is 13 bytes long on 32 bit version and 25 bytes long on 64 bit. - const size_t sentinel_block_length = /*check_flag(flags, e_flag_field_size_64_bit) ? (sizeof(uint64_t) * 3 + 1) : */(sizeof(uint32_t) * 3 + 1); + const size_t sentinel_block_length = version == 7500 ? (sizeof(uint64_t)* 3 + 1) : (sizeof(uint32_t)* 3 + 1); if (Offset(input, cursor) < end_offset) { if (end_offset - Offset(input, cursor) < sentinel_block_length) { @@ -403,7 +403,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, // XXX this is vulnerable to stack overflowing .. while(Offset(input, cursor) < end_offset - sentinel_block_length) { - ReadScope(output_tokens, input, cursor, input + end_offset - sentinel_block_length, flags); + ReadScope(output_tokens, input, cursor, input + end_offset - sentinel_block_length, version); } output_tokens.push_back(new_Token(cursor, cursor + 1, TokenType_CLOSE_BRACKET, Offset(input, cursor) )); @@ -438,19 +438,16 @@ void TokenizeBinary(TokenList& output_tokens, const char* input, unsigned int le TokenizeError("magic bytes not found",0); } - - //uint32_t offset = 0x15; - const char* cursor = input + 0x15; - - const uint32_t flags = ReadWord(input, cursor, input + length); - - const uint8_t padding_0 = ReadByte(input, cursor, input + length); // unused - (void) padding_0; - const uint8_t padding_1 = ReadByte(input, cursor, input + length); // unused - (void) padding_1; + const char* cursor = input + 18; + const uint8_t unknown_1 = ReadByte(input, cursor, input + length); + const uint8_t unknown_2 = ReadByte(input, cursor, input + length); + const uint8_t unknown_3 = ReadByte(input, cursor, input + length); + const uint8_t unknown_4 = ReadByte(input, cursor, input + length); + const uint8_t unknown_5 = ReadByte(input, cursor, input + length); + const uint32_t version = ReadWord(input, cursor, input + length); while (cursor < input + length) { - if(!ReadScope(output_tokens, input, cursor, input + length, flags)) { + if (!ReadScope(output_tokens, input, cursor, input + length, version)) { break; } } From 147541ab7f8eeffd2828530703cd8dfe792e8248 Mon Sep 17 00:00:00 2001 From: rickomax Date: Wed, 19 Jul 2017 19:04:10 -0300 Subject: [PATCH 2/2] Complementing last fix Complementing last fix --- code/FBXBinaryTokenizer.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/code/FBXBinaryTokenizer.cpp b/code/FBXBinaryTokenizer.cpp index fb57b2f97..d388c6446 100644 --- a/code/FBXBinaryTokenizer.cpp +++ b/code/FBXBinaryTokenizer.cpp @@ -340,11 +340,10 @@ void ReadData(const char*& sbegin_out, const char*& send_out, const char* input, // ------------------------------------------------------------------------------------------------ -//TODO: Test FBX Binary files newer than the 7500 version to check if the 64 bits address behaviour is consistent -bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, const char* end, uint32_t const version) +bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, const char* end, bool const is64bits) { // the first word contains the offset at which this block ends - const uint64_t end_offset = version == 7500 ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); + const uint64_t end_offset = is64bits ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); // we may get 0 if reading reached the end of the file - // fbx files have a mysterious extra footer which I don't know @@ -362,10 +361,10 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, } // the second data word contains the number of properties in the scope - const uint64_t prop_count = version == 7500 ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); + const uint64_t prop_count = is64bits ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); // the third data word contains the length of the property list - const uint64_t prop_length = version == 7500 ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); + const uint64_t prop_length = is64bits ? ReadDoubleWord(input, cursor, end) : ReadWord(input, cursor, end); // now comes the name of the scope/key const char* sbeg, *send; @@ -392,7 +391,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, // at the end of each nested block, there is a NUL record to indicate // that the sub-scope exists (i.e. to distinguish between P: and P : {}) // this NUL record is 13 bytes long on 32 bit version and 25 bytes long on 64 bit. - const size_t sentinel_block_length = version == 7500 ? (sizeof(uint64_t)* 3 + 1) : (sizeof(uint32_t)* 3 + 1); + const size_t sentinel_block_length = is64bits ? (sizeof(uint64_t)* 3 + 1) : (sizeof(uint32_t)* 3 + 1); if (Offset(input, cursor) < end_offset) { if (end_offset - Offset(input, cursor) < sentinel_block_length) { @@ -403,7 +402,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, // XXX this is vulnerable to stack overflowing .. while(Offset(input, cursor) < end_offset - sentinel_block_length) { - ReadScope(output_tokens, input, cursor, input + end_offset - sentinel_block_length, version); + ReadScope(output_tokens, input, cursor, input + end_offset - sentinel_block_length, is64bits); } output_tokens.push_back(new_Token(cursor, cursor + 1, TokenType_CLOSE_BRACKET, Offset(input, cursor) )); @@ -426,6 +425,7 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, } // ------------------------------------------------------------------------------------------------ +// TODO: Test FBX Binary files newer than the 7500 version to check if the 64 bits address behaviour is consistent void TokenizeBinary(TokenList& output_tokens, const char* input, unsigned int length) { ai_assert(input); @@ -445,9 +445,10 @@ void TokenizeBinary(TokenList& output_tokens, const char* input, unsigned int le const uint8_t unknown_4 = ReadByte(input, cursor, input + length); const uint8_t unknown_5 = ReadByte(input, cursor, input + length); const uint32_t version = ReadWord(input, cursor, input + length); + const bool is64bits = version == 7500; while (cursor < input + length) { - if (!ReadScope(output_tokens, input, cursor, input + length, version)) { + if (!ReadScope(output_tokens, input, cursor, input + length, is64bits)) { break; } }