diff --git a/hook/src/d3d9_hook.cpp b/hook/src/d3d9_hook.cpp index d4227c3..846a89f 100644 --- a/hook/src/d3d9_hook.cpp +++ b/hook/src/d3d9_hook.cpp @@ -366,11 +366,15 @@ bool install_d3d9_hooks(IpcClient& ipc) void remove_d3d9_hooks() { - // Reset the hook first (restores Present's bytes under thread suspension -> no new detour), - // then drain any Present detour still in-flight before freeing the D3D state it reads (UAF). - g_hk_present9 = {}; + // DISABLE (not destroy) first: restores Present's bytes under thread suspension (no new detour) + // but keeps the trampoline alive, so an in-flight detour about to call g_hk_present9.stdcall() + // (the trampoline) doesn't have it freed under it. Destroying (= {}) before the drain frees the + // trampoline immediately -- a UAF the uncapped mock-game storm (thousands of presents/s) hits + // reliably (0xC0000005). Disable -> drain -> only then destroy. + (void)g_hk_present9.disable(); hook_set_installed(g_id_present9, false); g_gate.drain(); + g_hk_present9 = {}; // no detour in-flight or able to start now -> safe to free the trampoline release_shared(); release_sysmem(); if (g_ctx != nullptr) diff --git a/hook/src/focus_spoof.cpp b/hook/src/focus_spoof.cpp index 688de3f..a653465 100644 --- a/hook/src/focus_spoof.cpp +++ b/hook/src/focus_spoof.cpp @@ -289,9 +289,17 @@ void remove_focus_spoof() SetWindowLongPtrA(g_game_hwnd, GWLP_WNDPROC, reinterpret_cast(g_orig_proc)); } } - g_focus_hooks.clear(); - g_hk_clipcursor = {}; // restore ClipCursor / SetCursorPos before clearing state - g_hk_setcursorpos = {}; + // Disable (restore original bytes) the inline focus hooks first so no new detour starts, but KEEP + // the trampolines alive for any in-flight detour calling its trampoline; destroy only after the + // drain. Destroying before the drain frees a trampoline under a detour about to call it (the same + // UAF class as the present-storm crash). The WNDPROC subclass uses a saved g_orig_proc pointer + // (restored above), not a trampoline, so it has nothing to free early. + for (auto& h : g_focus_hooks) + { + (void)h.disable(); + } + (void)g_hk_clipcursor.disable(); + (void)g_hk_setcursorpos.disable(); ClipCursor(nullptr); // leave the cursor free when the spoof is removed hook_set_installed(g_id_foreground, false); hook_set_installed(g_id_active, false); @@ -299,11 +307,14 @@ void remove_focus_spoof() hook_set_installed(g_id_wndproc, false); hook_set_installed(g_id_clipcursor, false); hook_set_installed(g_id_setcursorpos, false); - // The WNDPROC is restored and the inline hooks reset above, so no NEW detour can start. Drain + // The WNDPROC is restored and the inline hooks disabled above, so no NEW detour can start. Drain // any focus / WNDPROC detour still in-flight on the game's window thread before nulling the // state they read (g_orig_proc / g_game_hwnd / g_focus_ipc) -- otherwise a dispatch mid-flight // could call a null original WNDPROC or a dangling IPC pointer. g_gate.drain(); + g_focus_hooks.clear(); // no detour in-flight or able to start now -> safe to free the trampolines + g_hk_clipcursor = {}; + g_hk_setcursorpos = {}; if (g_focus_ipc != nullptr) { g_focus_ipc->mark_focus_spoof(false, 0); diff --git a/hook/src/hook_guard.hpp b/hook/src/hook_guard.hpp index ef6eb4b..67a8631 100644 --- a/hook/src/hook_guard.hpp +++ b/hook/src/hook_guard.hpp @@ -53,15 +53,23 @@ public: // Wait (bounded ~400 ms) for all in-flight detours to finish. Call AFTER the hook is // restored / the gate is closed, so no new detour can start -- otherwise this may never // reach zero on a busy render thread. + // + // We Sleep(1) *before* each zero-check (not after) to close an entry-window race: a thread can + // have already jumped into the detour but not yet reached its Guard constructor (the few- + // instruction prologue is unguarded), so m_active reads 0 even though a detour is about to run. + // Returning then would free the state out from under it. With the hook disabled no NEW detour can + // start, so any such thread reaches its Guard within nanoseconds; a 1 ms settle before concluding + // "zero" lets it register. Without this, a backend presenting at thousands/s (the uncapped + // mock-game storm) reliably crashed on remove (0xC0000005); with it, the count is accurate. void drain() { for (int spins = 0; spins < 400; ++spins) { + Sleep(1); if (m_active.load(std::memory_order_acquire) == 0) { return; } - Sleep(1); } } diff --git a/hook/src/mkb_hook.cpp b/hook/src/mkb_hook.cpp index b1ede67..f3d063b 100644 --- a/hook/src/mkb_hook.cpp +++ b/hook/src/mkb_hook.cpp @@ -514,12 +514,21 @@ void remove_mkb_hooks() return; } g_active.store(false, std::memory_order_release); - g_hk_async = {}; // InlineHook destructor restores the original bytes -> no new detour starts - g_hk_kbstate = {}; - g_hk_cursor = {}; - g_hk_getrawinputdata = {}; // restore GetRawInputData + // Disable (restore original bytes) the inline hooks first so no new detour starts, but KEEP the + // trampolines alive for any in-flight detour calling its trampoline; destroy only after the + // drain. Destroying before the drain frees the trampoline under a detour about to call it -- the + // same UAF class as the present-storm crash. (The DI hook is a vtable swap: remove() restores the + // slot and keeps m_original valid, so it has no trampoline to free early.) + (void)g_hk_async.disable(); + (void)g_hk_kbstate.disable(); + (void)g_hk_cursor.disable(); + (void)g_hk_getrawinputdata.disable(); g_vh_di_getstate.remove(); // restore the DI GetDeviceState slot (probe kept alive for re-enable) g_gate.drain(); // wait for any in-flight polling / DI / raw detour before clearing state + g_hk_async = {}; // no detour in-flight or able to start now -> safe to free the trampolines + g_hk_kbstate = {}; + g_hk_cursor = {}; + g_hk_getrawinputdata = {}; hook_set_installed(g_id_di_getstate, false); hook_set_installed(g_id_rawinput, false); for (int vk = 0; vk < 256; ++vk) diff --git a/hook/src/opengl_hook.cpp b/hook/src/opengl_hook.cpp index 532b22e..0938e0f 100644 --- a/hook/src/opengl_hook.cpp +++ b/hook/src/opengl_hook.cpp @@ -322,14 +322,18 @@ bool install_opengl_hooks(IpcClient& ipc) void remove_opengl_hooks() { - // Reset the hooks first (restores the original SwapBuffers bytes under thread suspension, so - // no NEW detour starts), then drain any swap detour still in-flight on the render thread - // BEFORE freeing the D3D state it reads -- otherwise the detour uses freed memory (UAF). - g_hk_swapbuffers = {}; - g_hk_wglswap = {}; + // DISABLE (not destroy) first: restores the SwapBuffers bytes under thread suspension (no new + // detour) but keeps the trampolines alive, so an in-flight detour about to call .stdcall() (the + // trampoline) doesn't have it freed under it. Destroying (= {}) before the drain frees the + // trampoline immediately -- a UAF the uncapped mock-game storm (thousands of swaps/s) can hit. + // Disable -> drain -> only then destroy. + (void)g_hk_swapbuffers.disable(); + (void)g_hk_wglswap.disable(); hook_set_installed(g_id_swapbuffers, false); hook_set_installed(g_id_wglswap, false); g_gate.drain(); + g_hk_swapbuffers = {}; // no detour in-flight or able to start now -> safe to free the trampolines + g_hk_wglswap = {}; release_shared(); if (g_ctx != nullptr) { diff --git a/hook/src/present_hook.cpp b/hook/src/present_hook.cpp index 407bf5c..5073b65 100644 --- a/hook/src/present_hook.cpp +++ b/hook/src/present_hook.cpp @@ -994,16 +994,22 @@ bool install_present_hooks(IpcClient& ipc) void remove_present_hooks() { - // Reset the hooks first (restores Present/Present1/ECL bytes under thread suspension, so no - // NEW detour starts), then drain any detour still in-flight on the render thread BEFORE - // freeing the shared texture / On12 bridge / present queue it reads -- otherwise UAF. - g_hk_present = {}; - g_hk_present1 = {}; - g_hk_ecl = {}; + // DISABLE (not destroy) the hooks first: this restores the Present/Present1/ECL bytes under + // thread suspension so no NEW detour starts, but KEEPS the trampolines alive -- an in-flight + // detour about to call g_hk_present.stdcall() (the trampoline) must not have it freed under it. + // Destroying here (= {}) frees the trampoline immediately; at a few hundred presents/s that race + // was rarely hit, but the uncapped mock-game storm (thousands/s) hits it reliably (0xC0000005). + // So: disable -> drain (in-flight detours finish on the live trampoline) -> only THEN destroy. + (void)g_hk_present.disable(); + (void)g_hk_present1.disable(); + (void)g_hk_ecl.disable(); hook_set_installed(g_id_present, false); hook_set_installed(g_id_present1, false); hook_set_installed(g_id_ecl, false); g_gate.drain(); + g_hk_present = {}; // no detour is in-flight or can start now -> freeing the trampoline is safe + g_hk_present1 = {}; + g_hk_ecl = {}; g_present_queue.store(nullptr, std::memory_order_relaxed); g_logged_presents_n = 0; // let a fresh injection re-log the present pattern g_logged_swapchains_n = 0; diff --git a/hook/src/vk_hook.cpp b/hook/src/vk_hook.cpp index 859095a..e350de5 100644 --- a/hook/src/vk_hook.cpp +++ b/hook/src/vk_hook.cpp @@ -334,8 +334,12 @@ void remove_vk_hooks() // game's thread BEFORE shutting the capture down. The game keeps calling our cached present // detour, but with the gate closed it now passes straight through to the real present. g_capture_enabled.store(false, std::memory_order_release); - g_hk_gipa = {}; + // Disable (not destroy) the GIPA hook first so the trampoline stays alive for any in-flight + // resolution detour calling real_gipa() (the trampoline); destroy only after the drain. (The + // present detour already passes through to the saved g_real_present, so it's unaffected.) + (void)g_hk_gipa.disable(); g_gate.drain(); + g_hk_gipa = {}; g_cap.shutdown(); // joins the reaper, drains the device, frees the read-back resources diff --git a/hook/src/xinput_hook.cpp b/hook/src/xinput_hook.cpp index 132151a..67db30c 100644 --- a/hook/src/xinput_hook.cpp +++ b/hook/src/xinput_hook.cpp @@ -234,12 +234,20 @@ bool install_xinput_hooks(IpcClient& ipc) void remove_xinput_hooks() { - g_hooks.clear(); // InlineHook destructor restores the original bytes -> no new detour starts + // Disable (restore original bytes) first so no new detour starts, but KEEP the trampolines alive + // for any in-flight detour calling its trampoline; destroy only after the drain. Clearing the + // vector here would free the trampolines immediately -- a UAF a game polling XInput at a high + // rate could hit (the same class as the mock-game present-storm crash). + for (auto& h : g_hooks) + { + (void)h.disable(); + } hook_set_installed(g_id_getstate, false); hook_set_installed(g_id_getstateex, false); hook_set_installed(g_id_getcaps, false); hook_set_installed(g_id_setstate, false); g_gate.drain(); // wait for any in-flight detour before nulling the IPC pointer it reads + g_hooks.clear(); // no detour in-flight or able to start now -> safe to free the trampolines if (g_ipc != nullptr) { g_ipc->mark_detached();