Fix: Improve memory store safety by avoiding unaligned stores

include/safetyhook/utility.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+#include <cstdint>
+
+namespace safetyhook {
+template <typename T> constexpr void store(uint8_t* address, const T& value) {
+    const auto data = reinterpret_cast<const uint8_t*>(&value);
+
+    // Write each byte out individually to avoid undefined behavior.
+    for (size_t i = 0; i < sizeof(T); ++i) {
+        address[i] = data[i];
+    }
+}
+} // namespace safetyhook

src/inline_hook.cpp

@@ -12,6 +12,7 @@
 
 #include <safetyhook/allocator.hpp>
 #include <safetyhook/thread_freezer.hpp>
+#include <safetyhook/utility.hpp>
 
 #include <safetyhook/inline_hook.hpp>
 
@@ -66,7 +67,7 @@ static auto make_jmp_ff(uint8_t* src, uint8_t* dst, uint8_t* data) {
     JmpFF jmp{};
 
     jmp.offset = static_cast<uint32_t>(data - src - sizeof(jmp));
-    *reinterpret_cast<uint8_t**>(data) = dst;
+    store(data, dst);
 
     return jmp;
 }
@@ -82,7 +83,7 @@ static void emit_jmp_ff(uint8_t* src, uint8_t* dst, uint8_t* data, size_t size =
         std::fill_n(src, size, static_cast<uint8_t>(0x90));
     }
 
-    *reinterpret_cast<JmpFF*>(src) = make_jmp_ff(src, dst, data);
+    store(src, make_jmp_ff(src, dst, data));
 }
 #endif
 
@@ -105,7 +106,7 @@ static void emit_jmp_e9(uint8_t* src, uint8_t* dst, size_t size = sizeof(JmpE9))
         std::fill_n(src, size, static_cast<uint8_t>(0x90));
     }
 
-    *reinterpret_cast<JmpE9*>(src) = make_jmp_e9(src, dst);
+    store(src, make_jmp_e9(src, dst));
 }
 
 static bool decode(ZydisDecodedInstruction* ix, uint8_t* ip) {
@@ -262,13 +263,13 @@ std::expected<void, InlineHook::Error> InlineHook::e9_hook(const std::shared_ptr
             std::copy_n(ip, ix.length, tramp_ip);
             const auto target_address = ip + ix.length + ix.raw.disp.value;
             const auto new_disp = target_address - (tramp_ip + ix.length);
-            *reinterpret_cast<int32_t*>(tramp_ip + ix.raw.disp.offset) = static_cast<int32_t>(new_disp);
+            store(tramp_ip + ix.raw.disp.offset, static_cast<int32_t>(new_disp));
             tramp_ip += ix.length;
         } else if (is_relative && ix.raw.imm[0].size == 32) {
             std::copy_n(ip, ix.length, tramp_ip);
             const auto target_address = ip + ix.length + ix.raw.imm[0].value.s;
             const auto new_disp = target_address - (tramp_ip + ix.length);
-            *reinterpret_cast<int32_t*>(tramp_ip + ix.raw.imm[0].offset) = static_cast<int32_t>(new_disp);
+            store(tramp_ip + ix.raw.imm[0].offset, static_cast<int32_t>(new_disp));
             tramp_ip += ix.length;
         } else if (ix.meta.category == ZYDIS_CATEGORY_COND_BR && ix.meta.branch_type == ZYDIS_BRANCH_TYPE_SHORT) {
             const auto target_address = ip + ix.length + ix.raw.imm[0].value.s;
@@ -281,7 +282,7 @@ std::expected<void, InlineHook::Error> InlineHook::e9_hook(const std::shared_ptr
 
             *tramp_ip = 0x0F;
             *(tramp_ip + 1) = 0x10 + ix.opcode;
-            *reinterpret_cast<int32_t*>(tramp_ip + 2) = static_cast<int32_t>(new_disp);
+            store(tramp_ip + 2, static_cast<int32_t>(new_disp));
             tramp_ip += 6;
         } else if (ix.meta.category == ZYDIS_CATEGORY_UNCOND_BR && ix.meta.branch_type == ZYDIS_BRANCH_TYPE_SHORT) {
             const auto target_address = ip + ix.length + ix.raw.imm[0].value.s;
@@ -293,7 +294,7 @@ std::expected<void, InlineHook::Error> InlineHook::e9_hook(const std::shared_ptr
             }
 
             *tramp_ip = 0xE9;
-            *reinterpret_cast<int32_t*>(tramp_ip + 1) = static_cast<int32_t>(new_disp);
+            store(tramp_ip + 1, static_cast<int32_t>(new_disp));
             tramp_ip += 5;
         } else {
             std::copy_n(ip, ix.length, tramp_ip);

src/mid_hook.cpp

@@ -2,6 +2,7 @@
 
 #include <safetyhook/allocator.hpp>
 #include <safetyhook/inline_hook.hpp>
+#include <safetyhook/utility.hpp>
 
 #include <safetyhook/mid_hook.hpp>
 
@@ -37,7 +38,8 @@ std::expected<MidHook, MidHook::Error> MidHook::create(
     const std::shared_ptr<Allocator>& allocator, uintptr_t target, MidHookFn destination) {
     MidHook hook{};
 
-    if (const auto setup_result = hook.setup(allocator, reinterpret_cast<uint8_t*>(target), destination); !setup_result) {
+    if (const auto setup_result = hook.setup(allocator, reinterpret_cast<uint8_t*>(target), destination);
+        !setup_result) {
         return std::unexpected{setup_result.error()};
     }
 
@@ -82,13 +84,13 @@ std::expected<void, MidHook::Error> MidHook::setup(
     std::copy_n(asm_data, sizeof(asm_data), reinterpret_cast<uint8_t*>(m_stub.address()));
 
 #ifdef _M_X64
-    *reinterpret_cast<MidHookFn*>(m_stub.address() + sizeof(asm_data) - 16) = m_destination;
+    store(m_stub.address() + sizeof(asm_data) - 16, m_destination);
 #else
-    *reinterpret_cast<MidHookFn*>(m_stub.address() + sizeof(asm_data) - 8) = m_destination;
+    store(m_stub.address() + sizeof(asm_data) - 8, m_destination);
 
     // 32-bit has some relocations we need to fix up as well.
-    *reinterpret_cast<uint8_t**>(m_stub.address() + 0xA + 2) = m_stub.address() + sizeof(asm_data) - 8;
-    *reinterpret_cast<uint8_t**>(m_stub.address() + 0x1C + 2) = m_stub.address() + sizeof(asm_data) - 4;
+    store(m_stub.address() + 0xA + 2, m_stub.address() + sizeof(asm_data) - 8);
+    store(m_stub.address() + 0x1C + 2, m_stub.address() + sizeof(asm_data) - 4);
 #endif
 
     auto hook_result = InlineHook::create(allocator, m_target, m_stub.address());
@@ -101,9 +103,9 @@ std::expected<void, MidHook::Error> MidHook::setup(
     m_hook = std::move(*hook_result);
 
 #ifdef _M_X64
-    *reinterpret_cast<uint8_t**>(m_stub.address() + sizeof(asm_data) - 8) = m_hook.trampoline().address();
+    store(m_stub.address() + sizeof(asm_data) - 8, m_hook.trampoline().address());
 #else
-    *reinterpret_cast<uint8_t**>(m_stub.address() + sizeof(asm_data) - 4) = m_hook.trampoline().address();
+    store(m_stub.address() + sizeof(asm_data) - 4, m_hook.trampoline().address());
 #endif
 
     return {};