Avoid raw reinterpret_casts in the FBX parser

As reinterpret_cast can break strict aliasing rules, causing runtime failure
on Android, replace such usage in FBXParser with memcpy.  Also provide a
utility routine for both performing the copy and asserting the validity of
the buffer length relative to the copied region.
pull/334/head
Jared Duke 2014-08-17 08:41:45 -07:00
parent 0ede630b10
commit 69810a2a07
1 changed files with 21 additions and 31 deletions

View File

@ -113,6 +113,18 @@ namespace {
ParseError(message); ParseError(message);
} }
// Initially, we did reinterpret_cast, breaking strict aliasing rules.
// This actually caused trouble on Android, so let's be safe this time.
// https://github.com/assimp/assimp/issues/24
template <typename T>
T SafeParse(const char* data, const char* end) {
// Actual size validation happens during Tokenization so
// this is valid as an assertion.
ai_assert(static_cast<size_t>(end - data) >= sizeof(T));
T result = static_cast<T>(0);
::memcpy(&result, data, sizeof(T));
return result;
}
} }
namespace Assimp { namespace Assimp {
@ -275,9 +287,7 @@ uint64_t ParseTokenAsID(const Token& t, const char*& err_out)
return 0L; return 0L;
} }
ai_assert(t.end() - data == 9); BE_NCONST uint64_t id = SafeParse<uint64_t>(data+1, t.end());
BE_NCONST uint64_t id = *reinterpret_cast<const uint64_t*>(data+1);
AI_SWAP8(id); AI_SWAP8(id);
return id; return id;
} }
@ -316,8 +326,7 @@ size_t ParseTokenAsDim(const Token& t, const char*& err_out)
return 0; return 0;
} }
ai_assert(t.end() - data == 9); BE_NCONST uint64_t id = SafeParse<uint64_t>(data+1, t.end());
BE_NCONST uint64_t id = *reinterpret_cast<const uint64_t*>(data+1);
AI_SWAP8(id); AI_SWAP8(id);
return static_cast<size_t>(id); return static_cast<size_t>(id);
} }
@ -364,24 +373,10 @@ float ParseTokenAsFloat(const Token& t, const char*& err_out)
} }
if (data[0] == 'F') { if (data[0] == 'F') {
// Actual size validation happens during Tokenization so return SafeParse<float>(data+1, t.end());
// this is valid as an assertion.
ai_assert(t.end() - data == sizeof(float) + 1);
// Initially, we did reinterpret_cast, breaking strict aliasing rules.
// This actually caused trouble on Android, so let's be safe this time.
// https://github.com/assimp/assimp/issues/24
float out_float;
::memcpy(&out_float, data+1, sizeof(float));
return out_float;
} }
else { else {
ai_assert(t.end() - data == sizeof(double) + 1); return SafeParse<double>(data+1, t.end());
// Same
double out_double;
::memcpy(&out_double, data+1, sizeof(double));
return static_cast<float>(out_double);
} }
} }
@ -416,8 +411,7 @@ int ParseTokenAsInt(const Token& t, const char*& err_out)
return 0; return 0;
} }
ai_assert(t.end() - data == 5); BE_NCONST int32_t ival = SafeParse<int32_t>(data+1, t.end());
BE_NCONST int32_t ival = *reinterpret_cast<const int32_t*>(data+1);
AI_SWAP4(ival); AI_SWAP4(ival);
return static_cast<int>(ival); return static_cast<int>(ival);
} }
@ -453,10 +447,8 @@ std::string ParseTokenAsString(const Token& t, const char*& err_out)
return ""; return "";
} }
ai_assert(t.end() - data >= 5);
// read string length // read string length
BE_NCONST int32_t len = *reinterpret_cast<const int32_t*>(data+1); BE_NCONST int32_t len = SafeParse<int32_t>(data+1, t.end());
AI_SWAP4(len); AI_SWAP4(len);
ai_assert(t.end() - data == 5 + len); ai_assert(t.end() - data == 5 + len);
@ -494,7 +486,7 @@ void ReadBinaryDataArrayHead(const char*& data, const char* end, char& type, uin
type = *data; type = *data;
// read number of elements // read number of elements
BE_NCONST uint32_t len = *reinterpret_cast<const uint32_t*>(data+1); BE_NCONST uint32_t len = SafeParse<uint32_t>(data+1, end);
AI_SWAP4(len); AI_SWAP4(len);
count = len; count = len;
@ -508,14 +500,12 @@ void ReadBinaryDataArray(char type, uint32_t count, const char*& data, const cha
std::vector<char>& buff, std::vector<char>& buff,
const Element& el) const Element& el)
{ {
ai_assert(static_cast<size_t>(end-data) >= 4); // runtime check for this happens at tokenization stage BE_NCONST uint32_t encmode = SafeParse<uint32_t>(data, end);
BE_NCONST uint32_t encmode = *reinterpret_cast<const uint32_t*>(data);
AI_SWAP4(encmode); AI_SWAP4(encmode);
data += 4; data += 4;
// next comes the compressed length // next comes the compressed length
BE_NCONST uint32_t comp_len = *reinterpret_cast<const uint32_t*>(data); BE_NCONST uint32_t comp_len = SafeParse<uint32_t>(data, end);
AI_SWAP4(comp_len); AI_SWAP4(comp_len);
data += 4; data += 4;