From 7740bbbaab58bb7a540fc06c5145fd9cad3221d6 Mon Sep 17 00:00:00 2001
From: jadebenn <jonahebenn@yahoo.com>
Date: Sat, 23 Nov 2024 02:29:11 -0800
Subject: [PATCH] reinterpret_cast-based type-punning is almost always UB

---
 CMakeLists.txt                                   |  2 +-
 dCommon/AmfSerialize.cpp                         |  3 +--
 dCommon/BrickByBrickFix.cpp                      |  2 +-
 dCommon/GeneralUtils.cpp                         |  2 +-
 dScripts/02_server/Map/AG/NpcAgCourseStarter.cpp | 16 ++++++++++------
 5 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a2affdc0..60c4e3f8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -285,7 +285,7 @@ if(MSVC)
 	# add_compile_options("/W4")
 	# Want to enable warnings eventually, but WAY too much noise right now
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
-	add_compile_options("-Wuninitialized" "-Wold-style-cast")
+	add_compile_options("-Wuninitialized" "-Wold-style-cast" "-Wstrict-aliasing")
 else()
 	message(WARNING "Unknown compiler: '${CMAKE_CXX_COMPILER_ID}' - No warning flags enabled.")
 endif()
diff --git a/dCommon/AmfSerialize.cpp b/dCommon/AmfSerialize.cpp
index 2827e1d6..db417e49 100644
--- a/dCommon/AmfSerialize.cpp
+++ b/dCommon/AmfSerialize.cpp
@@ -159,8 +159,7 @@ void RakNet::BitStream::Write<AMFIntValue&>(AMFIntValue& value) {
 // Writes an AMFDoubleValue to BitStream
 template<>
 void RakNet::BitStream::Write<AMFDoubleValue&>(AMFDoubleValue& value) {
-	double d = value.GetValue();
-	WriteAMFU64(*this, *reinterpret_cast<uint64_t*>(&d));
+	WriteAMFU64(*this, std::bit_cast<uint64_t>(value.GetValue()));
 }
 
 // Writes an AMFStringValue to BitStream
diff --git a/dCommon/BrickByBrickFix.cpp b/dCommon/BrickByBrickFix.cpp
index b771c986..85f4a558 100644
--- a/dCommon/BrickByBrickFix.cpp
+++ b/dCommon/BrickByBrickFix.cpp
@@ -146,7 +146,7 @@ void WriteSd0Magic(char* input, uint32_t chunkSize) {
 	input[2] = '0';
 	input[3] = 0x01;
 	input[4] = 0xFF;
-	*reinterpret_cast<uint32_t*>(input + 5) = chunkSize; // Write the integer to the character array
+	std::memcpy(&input[5], &chunkSize, sizeof(uint32_t)); // Write the integer to the character array
 }
 
 bool CheckSd0Magic(std::istream& streamToCheck) {
diff --git a/dCommon/GeneralUtils.cpp b/dCommon/GeneralUtils.cpp
index 52287904..e254a23a 100644
--- a/dCommon/GeneralUtils.cpp
+++ b/dCommon/GeneralUtils.cpp
@@ -53,7 +53,7 @@ bool static _IsSuffixChar(const uint8_t c) {
 bool GeneralUtils::details::_NextUTF8Char(std::string_view& slice, uint32_t& out) {
 	const size_t rem = slice.length();
 	if (slice.empty()) return false;
-	const uint8_t* bytes = reinterpret_cast<const uint8_t*>(&slice.front());
+	const auto* bytes = &slice.front();
 	if (rem > 0) {
 		const uint8_t first = bytes[0];
 		if (first < 0x80) { // 1 byte character
diff --git a/dScripts/02_server/Map/AG/NpcAgCourseStarter.cpp b/dScripts/02_server/Map/AG/NpcAgCourseStarter.cpp
index 515603a0..5bd0ff2b 100644
--- a/dScripts/02_server/Map/AG/NpcAgCourseStarter.cpp
+++ b/dScripts/02_server/Map/AG/NpcAgCourseStarter.cpp
@@ -50,9 +50,9 @@ void NpcAgCourseStarter::OnMessageBoxResponse(Entity* self, Entity* sender, int3
 
 		if (data->values[1] != 0) return;
 
-		time_t startTime = std::time(0) + 4; // Offset for starting timer
+		const time_t startTime = std::time(0) + 4; // Offset for starting timer
 
-		data->values[1] = *reinterpret_cast<float*>(&startTime);
+		std::memcpy(&data->values[1], &startTime, sizeof(float));
 
 		Game::entityManager->SerializeEntity(self);
 	} else if (identifier == u"FootRaceCancel") {
@@ -80,10 +80,14 @@ void NpcAgCourseStarter::OnFireEventServerSide(Entity* self, Entity* sender, std
 			LWOOBJID_EMPTY, "", sender->GetSystemAddress());
 		scriptedActivityComponent->RemoveActivityPlayerData(sender->GetObjectID());
 	} else if (args == "course_finish") {
-		time_t endTime = std::time(0);
-		time_t finish = (endTime - *reinterpret_cast<time_t*>(&data->values[1]));
-
-		data->values[2] = *reinterpret_cast<float*>(&finish);
+		const time_t endTime = std::time(0);
+		
+		// Using memcpy since misaligned reads are UB
+		time_t startTime{};
+		std::memcpy(&startTime, &data->values[1], sizeof(time_t));
+		const time_t finish = (endTime - startTime);
+		
+		std::memcpy(&data->values[2], &finish, sizeof(float));
 
 		auto* missionComponent = sender->GetComponent<MissionComponent>();
 		if (missionComponent != nullptr) {