Merge pull request #334 from jdduke/fbx_binary_fix

Avoid raw reinterpret_casts in the FBX parser

code/FBXParser.cpp

@@ -113,6 +113,18 @@ namespace {
 		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 {
@@ -275,9 +287,7 @@ uint64_t ParseTokenAsID(const Token& t, const char*& err_out)
 			return 0L;
 		}
 
-		ai_assert(t.end() - data == 9);
-
-		BE_NCONST uint64_t id = *reinterpret_cast<const uint64_t*>(data+1);
+		BE_NCONST uint64_t id = SafeParse<uint64_t>(data+1, t.end());
 		AI_SWAP8(id);
 		return id;
 	}
@@ -316,8 +326,7 @@ size_t ParseTokenAsDim(const Token& t, const char*& err_out)
 			return 0;
 		}
 
-		ai_assert(t.end() - data == 9);
-		BE_NCONST uint64_t id = *reinterpret_cast<const uint64_t*>(data+1);
+		BE_NCONST uint64_t id = SafeParse<uint64_t>(data+1, t.end());
 		AI_SWAP8(id);
 		return static_cast<size_t>(id);
 	}
@@ -364,24 +373,10 @@ float ParseTokenAsFloat(const Token& t, const char*& err_out)
 		}
 
 		if (data[0] == 'F') {
-			// Actual size validation happens during Tokenization so
-			// 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;
+			return SafeParse<float>(data+1, t.end());
 		}
 		else {
-			ai_assert(t.end() - data == sizeof(double) + 1);
-			
-			// Same
-			double out_double;
-			::memcpy(&out_double, data+1, sizeof(double));
-			return static_cast<float>(out_double);
+			return SafeParse<double>(data+1, t.end());
 		}
 	}
 
@@ -416,8 +411,7 @@ int ParseTokenAsInt(const Token& t, const char*& err_out)
 			return 0;
 		}
 
-		ai_assert(t.end() - data == 5);
-		BE_NCONST int32_t ival = *reinterpret_cast<const int32_t*>(data+1);
+		BE_NCONST int32_t ival = SafeParse<int32_t>(data+1, t.end());
 		AI_SWAP4(ival);
 		return static_cast<int>(ival);
 	}
@@ -453,10 +447,8 @@ std::string ParseTokenAsString(const Token& t, const char*& err_out)
 			return "";
 		}
 
-		ai_assert(t.end() - data >= 5);
-
 		// 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_assert(t.end() - data == 5 + len);
@@ -494,7 +486,7 @@ void ReadBinaryDataArrayHead(const char*& data, const char* end, char& type, uin
 	type = *data;
 
 	// 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);
 
 	count = len;
@@ -508,14 +500,12 @@ void ReadBinaryDataArray(char type, uint32_t count, const char*& data, const cha
 	std::vector<char>& buff, 
 	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 = *reinterpret_cast<const uint32_t*>(data);
+	BE_NCONST uint32_t encmode = SafeParse<uint32_t>(data, end);
 	AI_SWAP4(encmode);
 	data += 4;
 
 	// 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);
 	data += 4;