From 322f0393d010152a7290c87c84536b7de2e46b43 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 16 Apr 2023 17:07:09 -0700 Subject: [PATCH 01/14] Refactor: Replace uintptr_t addresses with uint8_t* This fixes instances where unsigned integer overflow could occur when calculating new displacements and uint8_t*'s are generally just easier to work with. --- include/safetyhook/allocator.hpp | 2 +- include/safetyhook/inline_hook.hpp | 2 +- include/safetyhook/mid_hook.hpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/safetyhook/allocator.hpp b/include/safetyhook/allocator.hpp index fb88366..758d856 100644 --- a/include/safetyhook/allocator.hpp +++ b/include/safetyhook/allocator.hpp @@ -124,4 +124,4 @@ class Allocator final : public std::enable_shared_from_this { [[nodiscard]] static bool in_range( uint8_t* address, const std::vector& desired_addresses, size_t max_distance); }; -} // namespace safetyhook \ No newline at end of file +} // namespace safetyhook diff --git a/include/safetyhook/inline_hook.hpp b/include/safetyhook/inline_hook.hpp index f66b87f..d255d4e 100644 --- a/include/safetyhook/inline_hook.hpp +++ b/include/safetyhook/inline_hook.hpp @@ -287,4 +287,4 @@ class InlineHook final { void destroy(); }; -} // namespace safetyhook \ No newline at end of file +} // namespace safetyhook diff --git a/include/safetyhook/mid_hook.hpp b/include/safetyhook/mid_hook.hpp index c5574b4..fa3521c 100644 --- a/include/safetyhook/mid_hook.hpp +++ b/include/safetyhook/mid_hook.hpp @@ -128,4 +128,4 @@ class MidHook final { std::expected setup( const std::shared_ptr& allocator, uint8_t* target, MidHookFn destination); }; -} // namespace safetyhook \ No newline at end of file +} // namespace safetyhook From f759f5d73ae418f585e132a7bab839145c3ced51 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 16 Apr 2023 23:47:15 -0700 Subject: [PATCH 02/14] Fix: Improve memory store safety by avoiding unaligned stores --- include/safetyhook/utility.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/safetyhook/utility.hpp b/include/safetyhook/utility.hpp index 277266e..d25007d 100644 --- a/include/safetyhook/utility.hpp +++ b/include/safetyhook/utility.hpp @@ -7,4 +7,4 @@ namespace safetyhook { template constexpr void store(uint8_t* address, const T& value) { std::copy_n(reinterpret_cast(&value), sizeof(T), address); } -} // namespace safetyhook \ No newline at end of file +} // namespace safetyhook From a62e278d370bd69a16a19235da9beae04cf4f6ab Mon Sep 17 00:00:00 2001 From: cursey Date: Sat, 22 Apr 2023 14:40:59 -0700 Subject: [PATCH 03/14] VmtHook: Initial unittest concept --- CMakeLists.txt | 2 ++ unittest/vmt_hook.cpp | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 unittest/vmt_hook.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a8132a..fe3ec58 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -489,6 +489,7 @@ if(SAFETYHOOK_BUILD_TESTS) # build-tests "unittest/inline_hook.cpp" "unittest/inline_hook.x86_64.cpp" "unittest/mid_hook.cpp" + "unittest/vmt_hook.cpp" cmake.toml ) @@ -557,6 +558,7 @@ if(SAFETYHOOK_BUILD_TESTS) # build-tests "unittest/inline_hook.cpp" "unittest/inline_hook.x86_64.cpp" "unittest/mid_hook.cpp" + "unittest/vmt_hook.cpp" "amalgamated-dist/safetyhook.cpp" cmake.toml ) diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp new file mode 100644 index 0000000..aaf17f0 --- /dev/null +++ b/unittest/vmt_hook.cpp @@ -0,0 +1,37 @@ +#include +#include + +TEST_CASE("VMT hook an object instance", "[vmt_hook]") { + struct Target { + __declspec(noinline) virtual int add_42(int a) { + return a + 42; + } + }; + + Target target{}; + + REQUIRE(target.add_42(0) == 42); + + static SafetyHookVmt vmt; + static SafetyHookVm hook; + + struct Hook { + static int __thiscall add_42(Target* self, int a) { + return hook.thiscall(self, a) + 1337; + } + }; + + auto vmt_result = SafetyHookVmt::create(&target); + + REQUIRE(vmt_result); + + vmt = std::move(*vmt_result); + + auto hook_result = vmt.hook(0, Hook::add_42); + + REQUIRE(hook_result); + + hook = std::move(*hook_result); + + REQUIRE(target.add_42(1) == 1380); +} \ No newline at end of file From 203ce81444561a93b0eb80d1c13d19210633930b Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 23 Apr 2023 04:07:18 -0700 Subject: [PATCH 04/14] VmtHook: Start stubbing out the implementation --- CMakeLists.txt | 1 + include/safetyhook/inline_hook.hpp | 10 +++++----- include/safetyhook/vmt_hook.hpp | 28 ++++++++++++++++++++++++++++ src/vmt_hook.cpp | 25 +++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 include/safetyhook/vmt_hook.hpp create mode 100644 src/vmt_hook.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index fe3ec58..5e00841 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,6 +93,7 @@ set(safetyhook_SOURCES "src/inline_hook.cpp" "src/mid_hook.cpp" "src/thread_freezer.cpp" + "src/vmt_hook.cpp" cmake.toml ) diff --git a/include/safetyhook/inline_hook.hpp b/include/safetyhook/inline_hook.hpp index d255d4e..9e65830 100644 --- a/include/safetyhook/inline_hook.hpp +++ b/include/safetyhook/inline_hook.hpp @@ -216,7 +216,7 @@ class InlineHook final { /// @return The result of calling the original function. /// @note This function will use the default calling convention set by your compiler. /// @note This function is unsafe because it doesn't lock the mutex. Only use this if you don't care about unhook - // safety or are worried about the performance cost of locking the mutex. + /// safety or are worried about the performance cost of locking the mutex. template RetT unsafe_call(Args... args) { return original()(args...); } @@ -228,7 +228,7 @@ class InlineHook final { /// @return The result of calling the original function. /// @note This function will use the __cdecl calling convention. /// @note This function is unsafe because it doesn't lock the mutex. Only use this if you don't care about unhook - // safety or are worried about the performance cost of locking the mutex. + /// safety or are worried about the performance cost of locking the mutex. template RetT unsafe_ccall(Args... args) { return original()(args...); } @@ -240,7 +240,7 @@ class InlineHook final { /// @return The result of calling the original function. /// @note This function will use the __thiscall calling convention. /// @note This function is unsafe because it doesn't lock the mutex. Only use this if you don't care about unhook - // safety or are worried about the performance cost of locking the mutex. + /// safety or are worried about the performance cost of locking the mutex. template RetT unsafe_thiscall(Args... args) { return original()(args...); } @@ -252,7 +252,7 @@ class InlineHook final { /// @return The result of calling the original function. /// @note This function will use the __stdcall calling convention. /// @note This function is unsafe because it doesn't lock the mutex. Only use this if you don't care about unhook - // safety or are worried about the performance cost of locking the mutex. + /// safety or are worried about the performance cost of locking the mutex. template RetT unsafe_stdcall(Args... args) { return original()(args...); } @@ -264,7 +264,7 @@ class InlineHook final { /// @return The result of calling the original function. /// @note This function will use the __fastcall calling convention. /// @note This function is unsafe because it doesn't lock the mutex. Only use this if you don't care about unhook - // safety or are worried about the performance cost of locking the mutex. + /// safety or are worried about the performance cost of locking the mutex. template RetT unsafe_fastcall(Args... args) { return original()(args...); } diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp new file mode 100644 index 0000000..b8078e0 --- /dev/null +++ b/include/safetyhook/vmt_hook.hpp @@ -0,0 +1,28 @@ +/// @file safetyhook/vmt_hook.hpp +/// @brief VMT hooking classes + +#pragma once + +#include +#include + +namespace safetyhook { + +class VmtHook final { +public: + [[nodiscard]] static VmtHook create(void* object); + + void reset(); + + template + [[nodiscard]] T* hook(size_t index, T* new_function) { + auto old_function = m_new_vmt[index]; + m_new_vmt[index] = reinterpret_cast(new_function); + return reinterpret_cast(old_function); + } + +private: + uint8_t** m_original_vmt{}; + std::vector m_new_vmt{}; +}; +} diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp new file mode 100644 index 0000000..6a39fe2 --- /dev/null +++ b/src/vmt_hook.cpp @@ -0,0 +1,25 @@ +#include + +namespace safetyhook { +VmtHook VmtHook::create(void* object) { + VmtHook hook{}; + + hook.m_original_vmt = *reinterpret_cast(object); + + for (auto vmt = hook.m_original_vmt; *vmt; ++vmt) { + hook.m_new_vmt.push_back(*vmt); + } + + *reinterpret_cast(object) = hook.m_new_vmt.data(); + + return hook; +} + +void VmtHook::reset() { + if (m_original_vmt != nullptr) { + *reinterpret_cast(m_original_vmt) = m_original_vmt; + } +} + + +} \ No newline at end of file From c16087a81e5518d644c2293295deb8849f03ac84 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 23 Apr 2023 19:56:55 -0700 Subject: [PATCH 05/14] VmtHook: Initial MVP --- include/safetyhook.hpp | 3 ++ include/safetyhook/vmt_hook.hpp | 72 ++++++++++++++++++++++--- src/vmt_hook.cpp | 69 +++++++++++++++++++++--- unittest/vmt_hook.cpp | 93 +++++++++++++++++++++++++++------ 4 files changed, 207 insertions(+), 30 deletions(-) diff --git a/include/safetyhook.hpp b/include/safetyhook.hpp index 4baf2a6..1206908 100644 --- a/include/safetyhook.hpp +++ b/include/safetyhook.hpp @@ -4,9 +4,12 @@ #include #include #include +#include using SafetyHookContext = safetyhook::Context; using SafetyHookInline = safetyhook::InlineHook; using SafetyHookMid = safetyhook::MidHook; using SafetyInlineHook [[deprecated("Use SafetyHookInline instead.")]] = safetyhook::InlineHook; using SafetyMidHook [[deprecated("Use SafetyHookMid instead.")]] = safetyhook::MidHook; +using SafetyHookVmt = safetyhook::VmtHook; +using SafetyHookVm = safetyhook::VmHook; diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index b8078e0..798ef8b 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -4,25 +4,85 @@ #pragma once #include +#include #include namespace safetyhook { +class VmHook final { +public: + VmHook() = default; + VmHook(const VmHook&) = delete; + VmHook(VmHook&& other) noexcept; + VmHook& operator=(const VmHook&) = delete; + VmHook& operator=(VmHook&& other) noexcept; + ~VmHook(); + + void reset(); + + template [[nodiscard]] T original() const { return reinterpret_cast(m_original_vm); } + + template RetT call(Args... args) { + return original()(args...); + } + + template RetT ccall(Args... args) { + return original()(args...); + } + + template RetT thiscall(Args... args) { + return original()(args...); + } + + template RetT stdcall(Args... args) { + return original()(args...); + } + + template RetT fastcall(Args... args) { + return original()(args...); + } + +private: + friend class VmtHook; + + uint8_t* m_original_vm{}; + uint8_t* m_new_vm{}; + uint8_t** m_vmt_entry{}; + + void destroy(); +}; class VmtHook final { public: - [[nodiscard]] static VmtHook create(void* object); + class Error {}; + [[nodiscard]] static std::expected create(void* object); + + VmtHook() = default; + VmtHook(const VmtHook&) = delete; + VmtHook(VmtHook&& other) noexcept; + VmtHook& operator=(const VmtHook&) = delete; + VmtHook& operator=(VmtHook&& other) noexcept; + ~VmtHook(); void reset(); template - [[nodiscard]] T* hook(size_t index, T* new_function) { - auto old_function = m_new_vmt[index]; - m_new_vmt[index] = reinterpret_cast(new_function); - return reinterpret_cast(old_function); + requires std::is_function_v + [[nodiscard]] std::expected hook_method(size_t index, T* new_function) { + VmHook hook{}; + + hook.m_original_vm = m_new_vmt[index]; + hook.m_new_vm = reinterpret_cast(new_function); + hook.m_vmt_entry = &m_new_vmt[index]; + m_new_vmt[index] = hook.m_new_vm; + + return hook; } private: + void* m_object{}; uint8_t** m_original_vmt{}; std::vector m_new_vmt{}; + + void destroy(); }; -} +} // namespace safetyhook diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp index 6a39fe2..cebdf21 100644 --- a/src/vmt_hook.cpp +++ b/src/vmt_hook.cpp @@ -1,13 +1,46 @@ #include namespace safetyhook { -VmtHook VmtHook::create(void* object) { +VmHook::VmHook(VmHook&& other) noexcept { + *this = std::move(other); +} + +VmHook& VmHook::operator=(VmHook&& other) noexcept { + destroy(); + m_original_vm = other.m_original_vm; + m_new_vm = other.m_new_vm; + m_vmt_entry = other.m_vmt_entry; + other.m_original_vm = nullptr; + other.m_new_vm = nullptr; + other.m_vmt_entry = nullptr; + return *this; +} + +VmHook::~VmHook() { + destroy(); +} + +void VmHook::reset() { + *this = {}; +} + +void VmHook::destroy() { + if (m_original_vm != nullptr) { + *m_vmt_entry = m_original_vm; + m_original_vm = nullptr; + m_new_vm = nullptr; + m_vmt_entry = nullptr; + } +} + +std::expected VmtHook::create(void* object) { VmtHook hook{}; + hook.m_object = object; hook.m_original_vmt = *reinterpret_cast(object); - for (auto vmt = hook.m_original_vmt; *vmt; ++vmt) { - hook.m_new_vmt.push_back(*vmt); + for (auto vm = hook.m_original_vmt; *vm; ++vm) { + hook.m_new_vmt.push_back(*vm); } *reinterpret_cast(object) = hook.m_new_vmt.data(); @@ -15,11 +48,33 @@ VmtHook VmtHook::create(void* object) { return hook; } +VmtHook::VmtHook(VmtHook&& other) noexcept { + *this = std::move(other); +} + +VmtHook& VmtHook::operator=(VmtHook&& other) noexcept { + destroy(); + m_object = other.m_object; + m_original_vmt = other.m_original_vmt; + m_new_vmt = std::move(other.m_new_vmt); + other.m_object = nullptr; + other.m_original_vmt = nullptr; + return *this; +} + +VmtHook::~VmtHook() { + destroy(); +} + void VmtHook::reset() { + *this = {}; +} + +void VmtHook::destroy() { if (m_original_vmt != nullptr) { - *reinterpret_cast(m_original_vmt) = m_original_vmt; + *reinterpret_cast(m_object) = m_original_vmt; + m_original_vmt = nullptr; + m_new_vmt.clear(); } } - - -} \ No newline at end of file +} // namespace safetyhook \ No newline at end of file diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index aaf17f0..9032bcd 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -2,36 +2,95 @@ #include TEST_CASE("VMT hook an object instance", "[vmt_hook]") { - struct Target { - __declspec(noinline) virtual int add_42(int a) { - return a + 42; - } + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; }; - Target target{}; + struct Target : public Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + }; + + std::unique_ptr target = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + + struct Hook { + static int __thiscall add_42(Target* self, int a) { return add_42_hook.thiscall(self, a) + 1337; } + }; + + auto vmt_result = SafetyHookVmt::create(target.get()); + + REQUIRE(vmt_result); + + target_hook = std::move(*vmt_result); + + auto vm_result = target_hook.hook_method(1, Hook::add_42); + + REQUIRE(vm_result); + + add_42_hook = std::move(*vm_result); - REQUIRE(target.add_42(0) == 42); + REQUIRE(target->add_42(1) == 1380); - static SafetyHookVmt vmt; - static SafetyHookVm hook; + add_42_hook.reset(); + + REQUIRE(target->add_42(2) == 44); +} + +TEST_CASE("Resetting the VMT hook removes all VM hooks for that object", "[vmt_hook]") { + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; + virtual int add_43(int a) = 0; + }; + + struct Target : public Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + __declspec(noinline) int add_43(int a) override { return a + 43; } + }; + + std::unique_ptr target = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + REQUIRE(target->add_43(0) == 43); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + static SafetyHookVm add_43_hook{}; struct Hook { - static int __thiscall add_42(Target* self, int a) { - return hook.thiscall(self, a) + 1337; - } + static int __thiscall add_42(Target* self, int a) { return add_42_hook.thiscall(self, a) + 1337; } + static int __thiscall add_43(Target* self, int a) { return add_43_hook.thiscall(self, a) + 1337; } }; - auto vmt_result = SafetyHookVmt::create(&target); + auto vmt_result = SafetyHookVmt::create(target.get()); REQUIRE(vmt_result); - vmt = std::move(*vmt_result); + target_hook = std::move(*vmt_result); + + auto vm_result = target_hook.hook_method(1, Hook::add_42); + + REQUIRE(vm_result); + + add_42_hook = std::move(*vm_result); + + REQUIRE(target->add_42(1) == 1380); + + vm_result = target_hook.hook_method(2, Hook::add_43); + + REQUIRE(vm_result); - auto hook_result = vmt.hook(0, Hook::add_42); + add_43_hook = std::move(*vm_result); - REQUIRE(hook_result); + REQUIRE(target->add_43(1) == 1381); - hook = std::move(*hook_result); + target_hook.reset(); - REQUIRE(target.add_42(1) == 1380); + REQUIRE(target->add_42(2) == 44); + REQUIRE(target->add_43(2) == 45); } \ No newline at end of file From 5b6d0fd05fc5b9ba3b405d8bfd050778ffda66f9 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 23 Apr 2023 20:40:22 -0700 Subject: [PATCH 06/14] VmtHook: Use a concept for function pointers NOTE: 32-bit MSVC doesn't like static __thiscall function pointers --- include/safetyhook/easy.hpp | 11 +++-------- include/safetyhook/inline_hook.hpp | 11 +++-------- include/safetyhook/mid_hook.hpp | 10 +++------- include/safetyhook/thread_freezer.hpp | 1 - include/safetyhook/utility.hpp | 12 ++++++++++++ include/safetyhook/vmt_hook.hpp | 6 +++--- unittest/vmt_hook.cpp | 4 ++-- 7 files changed, 26 insertions(+), 29 deletions(-) diff --git a/include/safetyhook/easy.hpp b/include/safetyhook/easy.hpp index 93d1c63..0666173 100644 --- a/include/safetyhook/easy.hpp +++ b/include/safetyhook/easy.hpp @@ -5,6 +5,7 @@ #include #include +#include namespace safetyhook { /// @brief Easy to use API for creating an InlineHook. @@ -14,13 +15,10 @@ namespace safetyhook { [[nodiscard]] InlineHook create_inline(void* target, void* destination); /// @brief Easy to use API for creating an InlineHook. -/// @tparam T The type of the function to hook. /// @param target The address of the function to hook. /// @param destination The address of the destination function. /// @return The InlineHook object. -template - requires std::is_function_v -[[nodiscard]] InlineHook create_inline(T* target, T* destination) { +[[nodiscard]] InlineHook create_inline(FnPtr auto target, FnPtr auto destination) { return create_inline(reinterpret_cast(target), reinterpret_cast(destination)); } @@ -31,13 +29,10 @@ template [[nodiscard]] MidHook create_mid(void* target, MidHookFn destination); /// @brief Easy to use API for creating a MidHook. -/// @tparam T The type of the function to hook. /// @param target the address of the function to hook. /// @param destination The destination function. /// @return The MidHook object. -template - requires std::is_function_v -[[nodiscard]] MidHook create_mid(T* target, MidHookFn destination) { +[[nodiscard]] MidHook create_mid(FnPtr auto target, MidHookFn destination) { return create_mid(reinterpret_cast(target), destination); } } // namespace safetyhook \ No newline at end of file diff --git a/include/safetyhook/inline_hook.hpp b/include/safetyhook/inline_hook.hpp index 9e65830..b773d99 100644 --- a/include/safetyhook/inline_hook.hpp +++ b/include/safetyhook/inline_hook.hpp @@ -11,6 +11,7 @@ #include #include +#include namespace safetyhook { /// @brief An inline hook. @@ -78,15 +79,12 @@ class InlineHook final { [[nodiscard]] static std::expected create(void* target, void* destination); /// @brief Create an inline hook. - /// @tparam T The type of the function to hook. /// @param target The address of the function to hook. /// @param destination The destination address. /// @return The InlineHook or an InlineHook::Error if an error occurred. /// @note This will use the default global Allocator. /// @note If you don't care about error handling, use the easy API (safetyhook::create_inline). - template - requires std::is_function_v - [[nodiscard]] static std::expected create(T* target, T* destination) { + [[nodiscard]] static std::expected create(FnPtr auto target, FnPtr auto destination) { return create(reinterpret_cast(target), reinterpret_cast(destination)); } @@ -100,16 +98,13 @@ class InlineHook final { const std::shared_ptr& allocator, void* target, void* destination); /// @brief Create an inline hook with a given Allocator. - /// @tparam T The type of the function to hook. /// @param allocator The allocator to use. /// @param target The address of the function to hook. /// @param destination The destination address. /// @return The InlineHook or an InlineHook::Error if an error occurred. /// @note If you don't care about error handling, use the easy API (safetyhook::create_inline). - template - requires std::is_function_v [[nodiscard]] static std::expected create( - const std::shared_ptr& allocator, T* target, T* destination) { + const std::shared_ptr& allocator, FnPtr auto target, FnPtr auto destination) { return create(allocator, reinterpret_cast(target), reinterpret_cast(destination)); } diff --git a/include/safetyhook/mid_hook.hpp b/include/safetyhook/mid_hook.hpp index fa3521c..4aa927c 100644 --- a/include/safetyhook/mid_hook.hpp +++ b/include/safetyhook/mid_hook.hpp @@ -9,6 +9,7 @@ #include #include #include +#include namespace safetyhook { @@ -56,15 +57,12 @@ class MidHook final { [[nodiscard]] static std::expected create(void* target, MidHookFn destination); /// @brief Creates a new MidHook object. - /// @tparam T The type of the function to hook. /// @param target The address of the function to hook. /// @param destination The destination function. /// @return The MidHook object or a MidHook::Error if an error occurred. /// @note This will use the default global Allocator. /// @note If you don't care about error handling, use the easy API (safetyhook::create_mid). - template - requires std::is_function_v - [[nodiscard]] static std::expected create(T* target, MidHookFn destination) { + [[nodiscard]] static std::expected create(FnPtr auto target, MidHookFn destination) { return create(reinterpret_cast(target), destination); } @@ -84,10 +82,8 @@ class MidHook final { /// @param destination The destination function. /// @return The MidHook object or a MidHook::Error if an error occurred. /// @note If you don't care about error handling, use the easy API (safetyhook::create_mid). - template - requires std::is_function_v [[nodiscard]] static std::expected create( - const std::shared_ptr& allocator, T* target, MidHookFn destination) { + const std::shared_ptr& allocator, FnPtr auto target, MidHookFn destination) { return create(allocator, reinterpret_cast(target), destination); } diff --git a/include/safetyhook/thread_freezer.hpp b/include/safetyhook/thread_freezer.hpp index 4cd5127..9f5d45f 100644 --- a/include/safetyhook/thread_freezer.hpp +++ b/include/safetyhook/thread_freezer.hpp @@ -5,7 +5,6 @@ #include #include -#include #include diff --git a/include/safetyhook/utility.hpp b/include/safetyhook/utility.hpp index d25007d..be4730e 100644 --- a/include/safetyhook/utility.hpp +++ b/include/safetyhook/utility.hpp @@ -1,10 +1,22 @@ #pragma once #include +#include #include namespace safetyhook { template constexpr void store(uint8_t* address, const T& value) { std::copy_n(reinterpret_cast(&value), sizeof(T), address); } + +template +concept FnPtr = requires(T f) { + std::is_pointer_v + + // 32-bit MSVC doesn't seem to think `static __thiscall` functions are functions. +#if defined(_M_X64) + && std::is_function_v> +#endif + ; +}; } // namespace safetyhook diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index 798ef8b..cc48af8 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -7,6 +7,8 @@ #include #include +#include + namespace safetyhook { class VmHook final { public: @@ -65,9 +67,7 @@ class VmtHook final { void reset(); - template - requires std::is_function_v - [[nodiscard]] std::expected hook_method(size_t index, T* new_function) { + [[nodiscard]] std::expected hook_method(size_t index, FnPtr auto new_function) { VmHook hook{}; hook.m_original_vm = m_new_vmt[index]; diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index 9032bcd..d82992e 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -7,7 +7,7 @@ TEST_CASE("VMT hook an object instance", "[vmt_hook]") { virtual int add_42(int a) = 0; }; - struct Target : public Interface { + struct Target : Interface { __declspec(noinline) int add_42(int a) override { return a + 42; } }; @@ -48,7 +48,7 @@ TEST_CASE("Resetting the VMT hook removes all VM hooks for that object", "[vmt_h virtual int add_43(int a) = 0; }; - struct Target : public Interface { + struct Target : Interface { __declspec(noinline) int add_42(int a) override { return a + 42; } __declspec(noinline) int add_43(int a) override { return a + 43; } }; From 8da27b758b3df63793ab88486d6352f6568927e3 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 23 Apr 2023 21:18:53 -0700 Subject: [PATCH 07/14] VmtHook: Maintain RTTI information when hooked --- include/safetyhook/utility.hpp | 2 +- include/safetyhook/vmt_hook.hpp | 1 + src/vmt_hook.cpp | 6 +++++- unittest/vmt_hook.cpp | 38 +++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/include/safetyhook/utility.hpp b/include/safetyhook/utility.hpp index be4730e..91508d2 100644 --- a/include/safetyhook/utility.hpp +++ b/include/safetyhook/utility.hpp @@ -1,8 +1,8 @@ #pragma once #include -#include #include +#include namespace safetyhook { template constexpr void store(uint8_t* address, const T& value) { diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index cc48af8..c6f0e5b 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -70,6 +70,7 @@ class VmtHook final { [[nodiscard]] std::expected hook_method(size_t index, FnPtr auto new_function) { VmHook hook{}; + ++index; // Skip RTTI pointer. hook.m_original_vm = m_new_vmt[index]; hook.m_new_vm = reinterpret_cast(new_function); hook.m_vmt_entry = &m_new_vmt[index]; diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp index cebdf21..f1e248e 100644 --- a/src/vmt_hook.cpp +++ b/src/vmt_hook.cpp @@ -39,11 +39,15 @@ std::expected VmtHook::create(void* object) { hook.m_object = object; hook.m_original_vmt = *reinterpret_cast(object); + // Copy pointer to RTTI. + hook.m_new_vmt.push_back(*(hook.m_original_vmt - 1)); + + // Copy virtual method pointers. for (auto vm = hook.m_original_vmt; *vm; ++vm) { hook.m_new_vmt.push_back(*vm); } - *reinterpret_cast(object) = hook.m_new_vmt.data(); + *reinterpret_cast(object) = hook.m_new_vmt.data() + 1; return hook; } diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index d82992e..81e6412 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -93,4 +93,42 @@ TEST_CASE("Resetting the VMT hook removes all VM hooks for that object", "[vmt_h REQUIRE(target->add_42(2) == 44); REQUIRE(target->add_43(2) == 45); +} + +TEST_CASE("VMT hooking an object maintains correct RTTI", "[vmt_hook]") { + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; + }; + + struct Target : Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + }; + + std::unique_ptr target = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + REQUIRE(dynamic_cast(target.get()) != nullptr); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + + struct Hook { + static int __thiscall add_42(Target* self, int a) { return add_42_hook.thiscall(self, a) + 1337; } + }; + + auto vmt_result = SafetyHookVmt::create(target.get()); + + REQUIRE(vmt_result); + + target_hook = std::move(*vmt_result); + + auto vm_result = target_hook.hook_method(1, Hook::add_42); + + REQUIRE(vm_result); + + add_42_hook = std::move(*vm_result); + + REQUIRE(target->add_42(1) == 1380); + REQUIRE(dynamic_cast(target.get()) != nullptr); } \ No newline at end of file From 746077375afd2f30e1cf776deec1a3076571cca1 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 23 Apr 2023 21:50:40 -0700 Subject: [PATCH 08/14] VmtHook: Have hooks inherit from their targets --- include/safetyhook/utility.hpp | 10 +--------- include/safetyhook/vmt_hook.hpp | 2 +- unittest/vmt_hook.cpp | 22 +++++++++++----------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/include/safetyhook/utility.hpp b/include/safetyhook/utility.hpp index 91508d2..3331ca0 100644 --- a/include/safetyhook/utility.hpp +++ b/include/safetyhook/utility.hpp @@ -10,13 +10,5 @@ template constexpr void store(uint8_t* address, const T& value) { } template -concept FnPtr = requires(T f) { - std::is_pointer_v - - // 32-bit MSVC doesn't seem to think `static __thiscall` functions are functions. -#if defined(_M_X64) - && std::is_function_v> -#endif - ; -}; +concept FnPtr = requires(T f) { std::is_pointer_v&& std::is_function_v>; }; } // namespace safetyhook diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index c6f0e5b..e4ec0c4 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -72,7 +72,7 @@ class VmtHook final { ++index; // Skip RTTI pointer. hook.m_original_vm = m_new_vmt[index]; - hook.m_new_vm = reinterpret_cast(new_function); + store(reinterpret_cast(&hook.m_new_vm), new_function); hook.m_vmt_entry = &m_new_vmt[index]; m_new_vmt[index] = hook.m_new_vm; diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index 81e6412..120759f 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -18,8 +18,8 @@ TEST_CASE("VMT hook an object instance", "[vmt_hook]") { static SafetyHookVmt target_hook{}; static SafetyHookVm add_42_hook{}; - struct Hook { - static int __thiscall add_42(Target* self, int a) { return add_42_hook.thiscall(self, a) + 1337; } + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } }; auto vmt_result = SafetyHookVmt::create(target.get()); @@ -28,7 +28,7 @@ TEST_CASE("VMT hook an object instance", "[vmt_hook]") { target_hook = std::move(*vmt_result); - auto vm_result = target_hook.hook_method(1, Hook::add_42); + auto vm_result = target_hook.hook_method(1, &Hook::hooked_add_42); REQUIRE(vm_result); @@ -62,9 +62,9 @@ TEST_CASE("Resetting the VMT hook removes all VM hooks for that object", "[vmt_h static SafetyHookVm add_42_hook{}; static SafetyHookVm add_43_hook{}; - struct Hook { - static int __thiscall add_42(Target* self, int a) { return add_42_hook.thiscall(self, a) + 1337; } - static int __thiscall add_43(Target* self, int a) { return add_43_hook.thiscall(self, a) + 1337; } + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } + int hooked_add_43(int a) { return add_43_hook.thiscall(this, a) + 1337; } }; auto vmt_result = SafetyHookVmt::create(target.get()); @@ -73,7 +73,7 @@ TEST_CASE("Resetting the VMT hook removes all VM hooks for that object", "[vmt_h target_hook = std::move(*vmt_result); - auto vm_result = target_hook.hook_method(1, Hook::add_42); + auto vm_result = target_hook.hook_method(1, &Hook::hooked_add_42); REQUIRE(vm_result); @@ -81,7 +81,7 @@ TEST_CASE("Resetting the VMT hook removes all VM hooks for that object", "[vmt_h REQUIRE(target->add_42(1) == 1380); - vm_result = target_hook.hook_method(2, Hook::add_43); + vm_result = target_hook.hook_method(2, &Hook::hooked_add_43); REQUIRE(vm_result); @@ -113,8 +113,8 @@ TEST_CASE("VMT hooking an object maintains correct RTTI", "[vmt_hook]") { static SafetyHookVmt target_hook{}; static SafetyHookVm add_42_hook{}; - struct Hook { - static int __thiscall add_42(Target* self, int a) { return add_42_hook.thiscall(self, a) + 1337; } + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } }; auto vmt_result = SafetyHookVmt::create(target.get()); @@ -123,7 +123,7 @@ TEST_CASE("VMT hooking an object maintains correct RTTI", "[vmt_hook]") { target_hook = std::move(*vmt_result); - auto vm_result = target_hook.hook_method(1, Hook::add_42); + auto vm_result = target_hook.hook_method(1, &Hook::hooked_add_42); REQUIRE(vm_result); From b0fc5d8ba25123941e90101435c27064c23fcd60 Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 30 Apr 2023 21:17:12 -0700 Subject: [PATCH 09/14] VmtHook: Use a shared Allocation to improve safety --- include/safetyhook/vmt_hook.hpp | 24 ++++++++++++-- src/vmt_hook.cpp | 56 +++++++++++++++++++++++++++------ unittest/vmt_hook.cpp | 39 +++++++++++++++++++++++ 3 files changed, 108 insertions(+), 11 deletions(-) diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index e4ec0c4..0280b10 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -7,6 +7,7 @@ #include #include +#include #include namespace safetyhook { @@ -50,12 +51,26 @@ class VmHook final { uint8_t* m_new_vm{}; uint8_t** m_vmt_entry{}; + // This keeps the allocation alive until the hook is destroyed. + std::shared_ptr m_new_vmt_allocation{}; + void destroy(); }; class VmtHook final { public: - class Error {}; + struct Error { + enum : uint8_t { BAD_ALLOCATION } type; + + union { + Allocator::Error allocator_error; + }; + + [[nodiscard]] static Error bad_allocation(Allocator::Error err) { + return {.type = BAD_ALLOCATION, .allocator_error = err}; + } + }; + [[nodiscard]] static std::expected create(void* object); VmtHook() = default; @@ -74,6 +89,7 @@ class VmtHook final { hook.m_original_vm = m_new_vmt[index]; store(reinterpret_cast(&hook.m_new_vm), new_function); hook.m_vmt_entry = &m_new_vmt[index]; + hook.m_new_vmt_allocation = m_new_vmt_allocation; m_new_vmt[index] = hook.m_new_vm; return hook; @@ -82,7 +98,11 @@ class VmtHook final { private: void* m_object{}; uint8_t** m_original_vmt{}; - std::vector m_new_vmt{}; + std::vector> m_vm_hooks{}; + + // The allocation is a shared_ptr, so it can be shared with VmHooks to ensure the memory is kept alive. + std::shared_ptr m_new_vmt_allocation{}; + uint8_t** m_new_vmt{}; void destroy(); }; diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp index f1e248e..1a49962 100644 --- a/src/vmt_hook.cpp +++ b/src/vmt_hook.cpp @@ -1,3 +1,8 @@ +#include + +#include +#include + #include namespace safetyhook { @@ -10,6 +15,7 @@ VmHook& VmHook::operator=(VmHook&& other) noexcept { m_original_vm = other.m_original_vm; m_new_vm = other.m_new_vm; m_vmt_entry = other.m_vmt_entry; + m_new_vmt_allocation = std::move(other.m_new_vmt_allocation); other.m_original_vm = nullptr; other.m_new_vm = nullptr; other.m_vmt_entry = nullptr; @@ -30,6 +36,7 @@ void VmHook::destroy() { m_original_vm = nullptr; m_new_vm = nullptr; m_vmt_entry = nullptr; + m_new_vmt_allocation.reset(); } } @@ -39,15 +46,30 @@ std::expected VmtHook::create(void* object) { hook.m_object = object; hook.m_original_vmt = *reinterpret_cast(object); + auto num_vmt_entries = 1; + + for (auto vm = hook.m_original_vmt; *vm; ++vm) { + ++num_vmt_entries; + } + + auto allocation = Allocator::global()->allocate(num_vmt_entries * sizeof(uint8_t*)); + + if (!allocation) { + return std::unexpected{Error::bad_allocation(allocation.error())}; + } + + hook.m_new_vmt_allocation = std::make_shared(std::move(*allocation)); + hook.m_new_vmt = reinterpret_cast(hook.m_new_vmt_allocation->data()); + // Copy pointer to RTTI. - hook.m_new_vmt.push_back(*(hook.m_original_vmt - 1)); + hook.m_new_vmt[0] = hook.m_original_vmt[-1]; // Copy virtual method pointers. - for (auto vm = hook.m_original_vmt; *vm; ++vm) { - hook.m_new_vmt.push_back(*vm); + for (auto i = 0; i < num_vmt_entries - 1; ++i) { + hook.m_new_vmt[i + 1] = hook.m_original_vmt[i]; } - *reinterpret_cast(object) = hook.m_new_vmt.data() + 1; + *reinterpret_cast(object) = &hook.m_new_vmt[1]; // hook.m_new_vmt.data() + 1; return hook; } @@ -60,9 +82,11 @@ VmtHook& VmtHook::operator=(VmtHook&& other) noexcept { destroy(); m_object = other.m_object; m_original_vmt = other.m_original_vmt; - m_new_vmt = std::move(other.m_new_vmt); + m_new_vmt_allocation = std::move(other.m_new_vmt_allocation); + m_new_vmt = other.m_new_vmt; other.m_object = nullptr; other.m_original_vmt = nullptr; + other.m_new_vmt = nullptr; return *this; } @@ -75,10 +99,24 @@ void VmtHook::reset() { } void VmtHook::destroy() { - if (m_original_vmt != nullptr) { - *reinterpret_cast(m_object) = m_original_vmt; - m_original_vmt = nullptr; - m_new_vmt.clear(); + if (m_original_vmt == nullptr) { + return; } + + execute_while_frozen([this] { + if (IsBadWritePtr(m_object, sizeof(void*))) { + return; + } + + if (*reinterpret_cast(m_object) != &m_new_vmt[1]) { + return; + } + + *reinterpret_cast(m_object) = m_original_vmt; + }); + + m_original_vmt = nullptr; + m_new_vmt_allocation.reset(); + m_new_vmt = nullptr; } } // namespace safetyhook \ No newline at end of file diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index 120759f..495e407 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -131,4 +131,43 @@ TEST_CASE("VMT hooking an object maintains correct RTTI", "[vmt_hook]") { REQUIRE(target->add_42(1) == 1380); REQUIRE(dynamic_cast(target.get()) != nullptr); +} + +TEST_CASE("Can safely destroy VmtHook after object is deleted", "[vmt_hook]") { + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; + }; + + struct Target : Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + }; + + std::unique_ptr target = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } + }; + + auto vmt_result = SafetyHookVmt::create(target.get()); + + REQUIRE(vmt_result); + + target_hook = std::move(*vmt_result); + + auto vm_result = target_hook.hook_method(1, &Hook::hooked_add_42); + + REQUIRE(vm_result); + + add_42_hook = std::move(*vm_result); + + REQUIRE(target->add_42(1) == 1380); + + target.reset(); + target_hook.reset(); } \ No newline at end of file From 97e586e85e4ea01e10ff64e6d8351563dc7c618d Mon Sep 17 00:00:00 2001 From: cursey Date: Sun, 30 Apr 2023 21:51:21 -0700 Subject: [PATCH 10/14] VmtHook: Add apply & remove to hook/unhook other object instances --- include/safetyhook/easy.hpp | 3 + include/safetyhook/vmt_hook.hpp | 10 ++- src/easy.cpp | 7 ++ src/vmt_hook.cpp | 65 +++++++++++----- unittest/vmt_hook.cpp | 127 ++++++++++++++++++++++++++++++++ 5 files changed, 188 insertions(+), 24 deletions(-) diff --git a/include/safetyhook/easy.hpp b/include/safetyhook/easy.hpp index 0666173..5379340 100644 --- a/include/safetyhook/easy.hpp +++ b/include/safetyhook/easy.hpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace safetyhook { /// @brief Easy to use API for creating an InlineHook. @@ -35,4 +36,6 @@ namespace safetyhook { [[nodiscard]] MidHook create_mid(FnPtr auto target, MidHookFn destination) { return create_mid(reinterpret_cast(target), destination); } + +[[nodiscard]] VmtHook create_vmt(void* object); } // namespace safetyhook \ No newline at end of file diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index 0280b10..0c1f2e9 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -5,7 +5,7 @@ #include #include -#include +#include #include #include @@ -80,6 +80,9 @@ class VmtHook final { VmtHook& operator=(VmtHook&& other) noexcept; ~VmtHook(); + void apply(void* object); + void remove(void* object); + void reset(); [[nodiscard]] std::expected hook_method(size_t index, FnPtr auto new_function) { @@ -96,9 +99,8 @@ class VmtHook final { } private: - void* m_object{}; - uint8_t** m_original_vmt{}; - std::vector> m_vm_hooks{}; + // Map of object instance to their original VMT. + std::unordered_map m_objects{}; // The allocation is a shared_ptr, so it can be shared with VmHooks to ensure the memory is kept alive. std::shared_ptr m_new_vmt_allocation{}; diff --git a/src/easy.cpp b/src/easy.cpp index 2508007..f79fbbb 100644 --- a/src/easy.cpp +++ b/src/easy.cpp @@ -17,4 +17,11 @@ MidHook create_mid(void* target, MidHookFn destination) { } } +VmtHook create_vmt(void* object) { + if (auto hook = VmtHook::create(object)) { + return std::move(*hook); + } else { + return {}; + } +} } // namespace safetyhook \ No newline at end of file diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp index 1a49962..249e6fa 100644 --- a/src/vmt_hook.cpp +++ b/src/vmt_hook.cpp @@ -1,6 +1,5 @@ #include -#include #include #include @@ -43,15 +42,17 @@ void VmHook::destroy() { std::expected VmtHook::create(void* object) { VmtHook hook{}; - hook.m_object = object; - hook.m_original_vmt = *reinterpret_cast(object); + const auto original_vmt = *reinterpret_cast(object); + hook.m_objects.emplace(object, original_vmt); + // Count the number of virtual method pointers. We start at one to account for the RTTI pointer. auto num_vmt_entries = 1; - for (auto vm = hook.m_original_vmt; *vm; ++vm) { + for (auto vm = original_vmt; *vm; ++vm) { ++num_vmt_entries; } + // Allocate memory for the new VMT. auto allocation = Allocator::global()->allocate(num_vmt_entries * sizeof(uint8_t*)); if (!allocation) { @@ -62,14 +63,14 @@ std::expected VmtHook::create(void* object) { hook.m_new_vmt = reinterpret_cast(hook.m_new_vmt_allocation->data()); // Copy pointer to RTTI. - hook.m_new_vmt[0] = hook.m_original_vmt[-1]; + hook.m_new_vmt[0] = original_vmt[-1]; // Copy virtual method pointers. for (auto i = 0; i < num_vmt_entries - 1; ++i) { - hook.m_new_vmt[i + 1] = hook.m_original_vmt[i]; + hook.m_new_vmt[i + 1] = original_vmt[i]; } - *reinterpret_cast(object) = &hook.m_new_vmt[1]; // hook.m_new_vmt.data() + 1; + *reinterpret_cast(object) = &hook.m_new_vmt[1]; return hook; } @@ -80,12 +81,9 @@ VmtHook::VmtHook(VmtHook&& other) noexcept { VmtHook& VmtHook::operator=(VmtHook&& other) noexcept { destroy(); - m_object = other.m_object; - m_original_vmt = other.m_original_vmt; + m_objects = std::move(other.m_objects); m_new_vmt_allocation = std::move(other.m_new_vmt_allocation); m_new_vmt = other.m_new_vmt; - other.m_object = nullptr; - other.m_original_vmt = nullptr; other.m_new_vmt = nullptr; return *this; } @@ -94,28 +92,55 @@ VmtHook::~VmtHook() { destroy(); } -void VmtHook::reset() { - *this = {}; +void VmtHook::apply(void* object) { + m_objects.emplace(object, *reinterpret_cast(object)); + *reinterpret_cast(object) = &m_new_vmt[1]; } -void VmtHook::destroy() { - if (m_original_vmt == nullptr) { +void VmtHook::remove(void* object) { + const auto search = m_objects.find(object); + + if (search == m_objects.end()) { return; } - execute_while_frozen([this] { - if (IsBadWritePtr(m_object, sizeof(void*))) { + const auto original_vmt = search->second; + + execute_while_frozen([&] { + if (IsBadWritePtr(object, sizeof(void*))) { return; } - if (*reinterpret_cast(m_object) != &m_new_vmt[1]) { + if (*reinterpret_cast(object) != &m_new_vmt[1]) { return; } - *reinterpret_cast(m_object) = m_original_vmt; + *reinterpret_cast(object) = original_vmt; + }); + + m_objects.erase(search); +} + +void VmtHook::reset() { + *this = {}; +} + +void VmtHook::destroy() { + execute_while_frozen([this] { + for (const auto [object, original_vmt] : m_objects) { + if (IsBadWritePtr(object, sizeof(void*))) { + return; + } + + if (*reinterpret_cast(object) != &m_new_vmt[1]) { + return; + } + + *reinterpret_cast(object) = original_vmt; + } }); - m_original_vmt = nullptr; + m_objects.clear(); m_new_vmt_allocation.reset(); m_new_vmt = nullptr; } diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index 495e407..e4c658a 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -170,4 +170,131 @@ TEST_CASE("Can safely destroy VmtHook after object is deleted", "[vmt_hook]") { target.reset(); target_hook.reset(); +} + +TEST_CASE("Can apply an existing VMT hook to more than one object", "[vmt_hook]") { + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; + }; + + struct Target : Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + }; + + std::unique_ptr target = std::make_unique(); + std::unique_ptr target0 = std::make_unique(); + std::unique_ptr target1 = std::make_unique(); + std::unique_ptr target2 = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } + }; + + auto vmt_result = SafetyHookVmt::create(target.get()); + + REQUIRE(vmt_result); + + target_hook = std::move(*vmt_result); + + auto vm_result = target_hook.hook_method(1, &Hook::hooked_add_42); + + REQUIRE(vm_result); + + add_42_hook = std::move(*vm_result); + + target_hook.apply(target0.get()); + target_hook.apply(target1.get()); + target_hook.apply(target2.get()); + + REQUIRE(target->add_42(1) == 1380); + REQUIRE(target0->add_42(1) == 1380); + REQUIRE(target1->add_42(1) == 1380); + REQUIRE(target2->add_42(1) == 1380); + + add_42_hook.reset(); + + REQUIRE(target->add_42(2) == 44); + REQUIRE(target0->add_42(2) == 44); + REQUIRE(target1->add_42(2) == 44); + REQUIRE(target2->add_42(2) == 44); +} + +TEST_CASE("Can remove an object that was previously VMT hooked", "[vmt_hook]") { + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; + }; + + struct Target : Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + }; + + std::unique_ptr target = std::make_unique(); + std::unique_ptr target0 = std::make_unique(); + std::unique_ptr target1 = std::make_unique(); + std::unique_ptr target2 = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } + }; + + auto vmt_result = SafetyHookVmt::create(target.get()); + + REQUIRE(vmt_result); + + target_hook = std::move(*vmt_result); + + auto vm_result = target_hook.hook_method(1, &Hook::hooked_add_42); + + REQUIRE(vm_result); + + add_42_hook = std::move(*vm_result); + + target_hook.apply(target0.get()); + target_hook.apply(target1.get()); + target_hook.apply(target2.get()); + + REQUIRE(target->add_42(1) == 1380); + REQUIRE(target0->add_42(1) == 1380); + REQUIRE(target1->add_42(1) == 1380); + REQUIRE(target2->add_42(1) == 1380); + + target_hook.remove(target0.get()); + + REQUIRE(target->add_42(2) == 1381); + REQUIRE(target0->add_42(2) == 44); + REQUIRE(target1->add_42(2) == 1381); + REQUIRE(target2->add_42(2) == 1381); + + target_hook.remove(target2.get()); + + REQUIRE(target->add_42(2) == 1381); + REQUIRE(target0->add_42(2) == 44); + REQUIRE(target1->add_42(2) == 1381); + REQUIRE(target2->add_42(2) == 44); + + target_hook.remove(target.get()); + + REQUIRE(target->add_42(2) == 44); + REQUIRE(target0->add_42(2) == 44); + REQUIRE(target1->add_42(2) == 1381); + REQUIRE(target2->add_42(2) == 44); + + target_hook.remove(target1.get()); + + REQUIRE(target->add_42(2) == 44); + REQUIRE(target0->add_42(2) == 44); + REQUIRE(target1->add_42(2) == 44); + REQUIRE(target2->add_42(2) == 44); } \ No newline at end of file From 8325e3e608026e772d0714f50c61fbfe51b6c08e Mon Sep 17 00:00:00 2001 From: cursey Date: Sat, 20 May 2023 21:43:40 -0700 Subject: [PATCH 11/14] VmtHook: Improve virtual method counting --- src/vmt_hook.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp index 249e6fa..954ec51 100644 --- a/src/vmt_hook.cpp +++ b/src/vmt_hook.cpp @@ -48,7 +48,7 @@ std::expected VmtHook::create(void* object) { // Count the number of virtual method pointers. We start at one to account for the RTTI pointer. auto num_vmt_entries = 1; - for (auto vm = original_vmt; *vm; ++vm) { + for (auto vm = original_vmt; !IsBadCodePtr(reinterpret_cast(*vm)); ++vm) { ++num_vmt_entries; } From ffff4c80a04f32731e6a17dee54ae9a7c7f305ec Mon Sep 17 00:00:00 2001 From: cursey Date: Tue, 13 Jun 2023 21:16:52 -0700 Subject: [PATCH 12/14] Utility: Add is_executable utility to replace IsBadCodePtr --- CMakeLists.txt | 1 + include/safetyhook/utility.hpp | 2 ++ src/utility.cpp | 50 ++++++++++++++++++++++++++++++++++ src/vmt_hook.cpp | 2 +- 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 src/utility.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e00841..6a5300b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,6 +93,7 @@ set(safetyhook_SOURCES "src/inline_hook.cpp" "src/mid_hook.cpp" "src/thread_freezer.cpp" + "src/utility.cpp" "src/vmt_hook.cpp" cmake.toml ) diff --git a/include/safetyhook/utility.hpp b/include/safetyhook/utility.hpp index 3331ca0..25b3707 100644 --- a/include/safetyhook/utility.hpp +++ b/include/safetyhook/utility.hpp @@ -11,4 +11,6 @@ template constexpr void store(uint8_t* address, const T& value) { template concept FnPtr = requires(T f) { std::is_pointer_v&& std::is_function_v>; }; + +bool is_executable(uint8_t* address); } // namespace safetyhook diff --git a/src/utility.cpp b/src/utility.cpp new file mode 100644 index 0000000..616204a --- /dev/null +++ b/src/utility.cpp @@ -0,0 +1,50 @@ +#include + +#include + +namespace safetyhook { +bool is_page_executable(uint8_t* address) { + MEMORY_BASIC_INFORMATION mbi; + + if (VirtualQuery(address, &mbi, sizeof(mbi)) == 0) { + return false; + } + + const auto executable_protect = PAGE_EXECUTE | PAGE_EXECUTE_READ | PAGE_EXECUTE_READWRITE | PAGE_EXECUTE_WRITECOPY; + + return (mbi.Protect & executable_protect) != 0; +} + +bool is_executable(uint8_t* address) { + LPVOID image_base_ptr; + + if (RtlPcToFileHeader(address, &image_base_ptr) == nullptr) { + return is_page_executable(address); + } + + // Just check if the section is executable. + const auto* image_base = reinterpret_cast(image_base_ptr); + const auto* dos_hdr = reinterpret_cast(image_base); + + if (dos_hdr->e_magic != IMAGE_DOS_SIGNATURE) { + return is_page_executable(address); + } + + const auto* nt_hdr = reinterpret_cast(image_base + dos_hdr->e_lfanew); + + if (nt_hdr->Signature != IMAGE_NT_SIGNATURE) { + return is_page_executable(address); + } + + const auto* section = IMAGE_FIRST_SECTION(nt_hdr); + + for (auto i = 0; i < nt_hdr->FileHeader.NumberOfSections; ++i, ++section) { + if (address >= image_base + section->VirtualAddress && + address < image_base + section->VirtualAddress + section->Misc.VirtualSize) { + return (section->Characteristics & IMAGE_SCN_MEM_EXECUTE) != 0; + } + } + + return is_page_executable(address); +} +} // namespace safetyhook \ No newline at end of file diff --git a/src/vmt_hook.cpp b/src/vmt_hook.cpp index 954ec51..cec6512 100644 --- a/src/vmt_hook.cpp +++ b/src/vmt_hook.cpp @@ -48,7 +48,7 @@ std::expected VmtHook::create(void* object) { // Count the number of virtual method pointers. We start at one to account for the RTTI pointer. auto num_vmt_entries = 1; - for (auto vm = original_vmt; !IsBadCodePtr(reinterpret_cast(*vm)); ++vm) { + for (auto vm = original_vmt; is_executable(*vm); ++vm) { ++num_vmt_entries; } From 5e671847caccf935382c0c98ed5c9255e7ce4fb1 Mon Sep 17 00:00:00 2001 From: cursey Date: Thu, 5 Oct 2023 18:27:37 -0700 Subject: [PATCH 13/14] VmtHook: Add easy API for hooking methods --- include/safetyhook/easy.hpp | 8 ++++++++ unittest/vmt_hook.cpp | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/include/safetyhook/easy.hpp b/include/safetyhook/easy.hpp index 5379340..dab67d2 100644 --- a/include/safetyhook/easy.hpp +++ b/include/safetyhook/easy.hpp @@ -38,4 +38,12 @@ namespace safetyhook { } [[nodiscard]] VmtHook create_vmt(void* object); +[[nodiscard]] VmHook create_vm(VmtHook& vmt, size_t index, FnPtr auto destination) { + if (auto hook = vmt.hook_method(index, destination)) { + return std::move(*hook); + } else { + return {}; + } +} + } // namespace safetyhook \ No newline at end of file diff --git a/unittest/vmt_hook.cpp b/unittest/vmt_hook.cpp index e4c658a..3365ebe 100644 --- a/unittest/vmt_hook.cpp +++ b/unittest/vmt_hook.cpp @@ -297,4 +297,35 @@ TEST_CASE("Can remove an object that was previously VMT hooked", "[vmt_hook]") { REQUIRE(target0->add_42(2) == 44); REQUIRE(target1->add_42(2) == 44); REQUIRE(target2->add_42(2) == 44); -} \ No newline at end of file +} + +TEST_CASE("VMT hook an object instance with easy API", "[vmt_hook]") { + struct Interface { + virtual ~Interface() = default; + virtual int add_42(int a) = 0; + }; + + struct Target : Interface { + __declspec(noinline) int add_42(int a) override { return a + 42; } + }; + + std::unique_ptr target = std::make_unique(); + + REQUIRE(target->add_42(0) == 42); + + static SafetyHookVmt target_hook{}; + static SafetyHookVm add_42_hook{}; + + struct Hook : Target { + int hooked_add_42(int a) { return add_42_hook.thiscall(this, a) + 1337; } + }; + + target_hook = safetyhook::create_vmt(target.get()); + add_42_hook = safetyhook::create_vm(target_hook, 1, &Hook::hooked_add_42); + + REQUIRE(target->add_42(1) == 1380); + + add_42_hook.reset(); + + REQUIRE(target->add_42(2) == 44); +} From 5609ea7ad1fb6db1688d2e227308018dc7cdcbdb Mon Sep 17 00:00:00 2001 From: cursey Date: Thu, 5 Oct 2023 19:23:08 -0700 Subject: [PATCH 14/14] VmtHook: Add documentation --- include/safetyhook/easy.hpp | 9 ++++++ include/safetyhook/vmt_hook.hpp | 55 +++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/include/safetyhook/easy.hpp b/include/safetyhook/easy.hpp index dab67d2..8e12f9f 100644 --- a/include/safetyhook/easy.hpp +++ b/include/safetyhook/easy.hpp @@ -37,7 +37,16 @@ namespace safetyhook { return create_mid(reinterpret_cast(target), destination); } +/// @brief Easy to use API for creating a VmtHook. +/// @param object The object to hook. +/// @return The VmtHook object. [[nodiscard]] VmtHook create_vmt(void* object); + +/// @brief Easy to use API for creating a VmHook. +/// @param vmt The VmtHook to use to create the VmHook. +/// @param index The index of the method to hook. +/// @param destination The destination function. +/// @return The VmHook object. [[nodiscard]] VmHook create_vm(VmtHook& vmt, size_t index, FnPtr auto destination) { if (auto hook = vmt.hook_method(index, destination)) { return std::move(*hook); diff --git a/include/safetyhook/vmt_hook.hpp b/include/safetyhook/vmt_hook.hpp index 0c1f2e9..f35d560 100644 --- a/include/safetyhook/vmt_hook.hpp +++ b/include/safetyhook/vmt_hook.hpp @@ -11,6 +11,7 @@ #include namespace safetyhook { +/// @brief A hook class that allows for hooking a single method in a VMT. class VmHook final { public: VmHook() = default; @@ -20,26 +21,54 @@ class VmHook final { VmHook& operator=(VmHook&& other) noexcept; ~VmHook(); + /// @brief Removes the hook. void reset(); + /// @brief Gets the original method pointer. template [[nodiscard]] T original() const { return reinterpret_cast(m_original_vm); } + /// @brief Calls the original method. + /// @tparam RetT The return type of the method. + /// @tparam Args The argument types of the method. + /// @param args The arguments to pass to the method. + /// @return The return value of the method. + /// @note This will call the original method with the default calling convention. template RetT call(Args... args) { return original()(args...); } + /// @brief Calls the original method with the __cdecl calling convention. + /// @tparam RetT The return type of the method. + /// @tparam Args The argument types of the method. + /// @param args The arguments to pass to the method. + /// @return The return value of the method. template RetT ccall(Args... args) { return original()(args...); } + /// @brief Calls the original method with the __thiscall calling convention. + /// @tparam RetT The return type of the method. + /// @tparam Args The argument types of the method. + /// @param args The arguments to pass to the method. + /// @return The return value of the method. template RetT thiscall(Args... args) { return original()(args...); } + /// @brief Calls the original method with the __stdcall calling convention. + /// @tparam RetT The return type of the method. + /// @tparam Args The argument types of the method. + /// @param args The arguments to pass to the method. + /// @return The return value of the method. template RetT stdcall(Args... args) { return original()(args...); } + /// @brief Calls the original method with the __fastcall calling convention. + /// @tparam RetT The return type of the method. + /// @tparam Args The argument types of the method. + /// @param args The arguments to pass to the method. + /// @return The return value of the method. template RetT fastcall(Args... args) { return original()(args...); } @@ -57,20 +86,32 @@ class VmHook final { void destroy(); }; +/// @brief A hook class that copies an entire VMT for a given object and replaces it. class VmtHook final { public: + /// @brief Error type for VmtHook. struct Error { - enum : uint8_t { BAD_ALLOCATION } type; + /// @brief The type of error. + enum : uint8_t { + BAD_ALLOCATION, ///< An error occurred while allocating memory. + } type; + /// @brief Extra error information. union { - Allocator::Error allocator_error; + Allocator::Error allocator_error; ///< Allocator error information. }; + /// @brief Create a BAD_ALLOCATION error. + /// @param err The Allocator::Error that failed. + /// @return The new BAD_ALLOCATION error. [[nodiscard]] static Error bad_allocation(Allocator::Error err) { return {.type = BAD_ALLOCATION, .allocator_error = err}; } }; + /// @brief Creates a new VmtHook object. Will clone the VMT of the given object and replace it. + /// @param object The object to hook. + /// @return The VmtHook object or a VmtHook::Error if an error occurred. [[nodiscard]] static std::expected create(void* object); VmtHook() = default; @@ -80,11 +121,21 @@ class VmtHook final { VmtHook& operator=(VmtHook&& other) noexcept; ~VmtHook(); + /// @brief Applies the hook. + /// @param object The object to apply the hook to. + /// @note This will replace the VMT of the object with the new VMT. You can apply the hook to multiple objects. void apply(void* object); + + /// @brief Removes the hook. + /// @param object The object to remove the hook from. void remove(void* object); + /// @brief Removes the hook from all objects. void reset(); + /// @brief Hooks a method in the VMT. + /// @param index The index of the method to hook. + /// @param new_function The new function to use. [[nodiscard]] std::expected hook_method(size_t index, FnPtr auto new_function) { VmHook hook{};