From b978790dce5c763f31bd6ad2fef59fad667c1e47 Mon Sep 17 00:00:00 2001 From: AlienDwarf <61460778+AlienDwarf@users.noreply.github.com> Date: Sat, 20 Jun 2026 02:09:05 +0200 Subject: [PATCH 1/5] feat: capture XMM and MXCSR state in MidHook context Extend the mid-function context bridge to snapshot the full SSE / floating-point state, not just the GPRs and flags, so handlers can observe and rewrite floating-point and SIMD register arguments in flight. The stub now spills XMM0..XMM15 (XMM0..XMM7 on x86) with movups and the MXCSR control/status word with stmxcsr before calling the handler, and reloads them (applying any edits) with movups / ldmxcsr afterwards. movups is used so the spill needs no 16-byte stack alignment. HookContext gains an xmm array, the mxcsr field and a new Xmm type; the save order keeps the block contiguous and the handler pointer aliasing the struct exactly, locked down by offset_of! layout tests. The legacy x87 stack registers remain out of scope (documented). Bumps the reserved stub capacity to 512 bytes to fit the larger save/restore block. Adds runtime tests that observe and rewrite a double argument through XMM0 on x86_64, and serializes the mid-hook integration tests (each install suspends all other threads, so parallel installs on shared targets could collide). --- README.md | 26 +++--- include/neohook.h | 67 +++++++++++-- include/neohook.hpp | 5 +- src/lib.rs | 2 +- src/midhook.rs | 222 +++++++++++++++++++++++++++++++++++++------- tests/midhook.rs | 83 +++++++++++++++++ 6 files changed, 348 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index fafcf02..3f3dd67 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ Function hooking is deceptively difficult to get right. Writing a `JMP` patch is - **Named Hook Registry** - Park hooks in a process-wide store and refer to them by name: `registry::register`, `enable` / `disable`, `unhook`, and `unhook_all` for a single teardown point (e.g. `DLL_PROCESS_DETACH`). -- **Mid-Function / Arbitrary-Address Detours** - Hook *any* instruction boundary, not just a function entry. NeoHook snapshots all general-purpose registers and flags into a `HookContext`, calls your handler with a pointer to it, restores the (possibly modified) registers, then resumes the original instructions. Rewrite arguments, results, or loop state in flight at a spot found by a signature scan - all on the thread-safe inline engine (thread suspension, IP/stack redirection, relocation, atomic rollback). +- **Mid-Function / Arbitrary-Address Detours** - Hook *any* instruction boundary, not just a function entry. NeoHook snapshots all general-purpose registers, flags, the XMM registers and `MXCSR` into a `HookContext`, calls your handler with a pointer to it, restores the (possibly modified) state, then resumes the original instructions. Rewrite integer or floating-point/SIMD arguments, results, or loop state in flight at a spot found by a signature scan - all on the thread-safe inline engine (thread suspension, IP/stack redirection, relocation, atomic rollback). - **VTable Hooking** - Rewrites a selected VTable slot to detour virtual calls and restores the original slot on unhook. @@ -124,7 +124,7 @@ Function hooking is deceptively difficult to get right. Writing a `JMP` patch is | v0.8.0 | ✅ Done | Closure detours (`detour_closure!`) | | v0.8.0 | ✅ Done | Delay / on-load hooks (`DelayHook`) | | v0.8.0 | ✅ Done | Named hook registry (`registry`) | -| v0.9.0 | Planned | XMM / FPU context capture in `MidHook` | +| v0.9.0 | ✅ Done | XMM / MXCSR context capture in `MidHook` | | v0.9.0 | Planned | Control-flow redirect from a `MidHook` handler | | v0.9.0 | Planned | ARM64 inline hooking | @@ -370,11 +370,12 @@ typically located with a [signature scan](#pattern--signature-scanning). Because such a site is reached with arbitrary registers live, a normal detour would clobber them. Instead NeoHook installs a context bridge: it snapshots all -general-purpose registers and flags into a [`HookContext`], calls your handler -with a pointer to it, restores the (possibly modified) registers, then runs the -original instructions and resumes the function. The patch runs on the full -inline engine - threads suspended, instruction pointers/return addresses -redirected, stolen bytes relocated, atomic rollback on failure. +general-purpose registers, flags, every XMM register and `MXCSR` into a +[`HookContext`], calls your handler with a pointer to it, restores the +(possibly modified) state, then runs the original instructions and resumes the +function. The patch runs on the full inline engine - threads suspended, +instruction pointers/return addresses redirected, stolen bytes relocated, +atomic rollback on failure. ```rust use neohook::{HookContext, MidHook}; @@ -402,10 +403,13 @@ fn main() { ``` A handler may read any field of `HookContext` to observe a live register, or -write one to change it before execution continues. The detour always *continues* -the original function - it cannot skip it or redirect control flow, and only -general-purpose registers and flags are captured (not XMM/FPU state). `target` -must sit on a real instruction boundary. +write one to change it before execution continues - including the floating-point +/ SIMD argument registers via `ctx.xmm[..]` (e.g. +`f64::from_bits(ctx.xmm[0].low)` for a scalar `double`). The detour always +*continues* the original function - it cannot skip it or redirect control flow. +General-purpose registers, flags, all XMM registers and `MXCSR` are captured; +the legacy x87 stack registers are not. `target` must sit on a real instruction +boundary. The C ABI exposes `detours_midhook_install(target, handler)` and `detours_midhook_unhook(hook)`; the C++ wrapper provides an RAII `neohook::MidHook`. diff --git a/include/neohook.h b/include/neohook.h index 30d5cf2..b9d8480 100644 --- a/include/neohook.h +++ b/include/neohook.h @@ -64,17 +64,41 @@ typedef struct MidHook MidHook; */ typedef struct VehHook VehHook; +/** + * A 128-bit XMM register, captured for a [`MidHook`] handler. + * + * The two halves are stored little-endian, matching how `movups` writes the + * register to memory: `low` is bytes 0..8 (a packed `f64`, the scalar `f32`, + * or the low quadword of a vector) and `high` is bytes 8..16. A handler reads + * or writes these directly - e.g. `f64::from_bits(ctx.xmm[0].low)` to read a + * scalar `double` argument, or `ctx.xmm[0].low = v.to_bits()` to replace it. + */ +typedef struct Xmm { + /** + * Bytes 0..8 of the register (the low quadword). + */ + uint64_t low; + /** + * Bytes 8..16 of the register (the high quadword). + */ + uint64_t high; +} Xmm; + #if defined(_M_X64) /** - * Snapshot of the general-purpose registers and flags at the hook site, - * captured for an x86_64 [`MidHook`] handler. + * Snapshot of the general-purpose registers, flags and SSE / floating-point + * state at the hook site, captured for an x86_64 [`MidHook`] handler. * - * The field order matches the order in which the stub pushes registers, so the + * The field order matches the order in which the stub saves the state, so the * pointer passed to the handler aliases this layout exactly. A handler may read * any field to observe the live value, or write any field to change the * register before the original instructions resume. `rsp` is captured for * inspection but writing it has no effect (the stack pointer is managed by the * stub). + * + * `xmm` holds `XMM0`..`XMM15` in order, and `mxcsr` holds the SSE + * control/status word. The x87 stack registers are not captured (see the + * module-level limits). */ typedef struct HookContext { /** @@ -96,16 +120,30 @@ typedef struct HookContext { uint64_t r13; uint64_t r14; uint64_t r15; + /** + * The SSE control/status register (`MXCSR`). + */ + uint32_t mxcsr; + /** + * Padding so `xmm` is 8-byte aligned; mirrors a reserved slot in the stub. + */ + uint32_t _reserved; + /** + * `XMM0` through `XMM15`, in register-number order. + */ + struct Xmm xmm[16]; } HookContext; #endif #if defined(_M_IX86) /** - * Snapshot of the general-purpose registers and flags at the hook site, - * captured for an x86 [`MidHook`] handler. + * Snapshot of the general-purpose registers, flags and SSE / floating-point + * state at the hook site, captured for an x86 [`MidHook`] handler. * - * The layout matches the `pushad` + `pushfd` block the stub writes. Writing - * `esp` has no effect (`popad` discards its saved stack pointer slot). + * The layout matches the `pushad` + `pushfd` block the stub writes, preceded + * by the saved `MXCSR` and `XMM0`..`XMM7`. Writing `esp` has no effect + * (`popad` discards its saved stack pointer slot). The x87 stack registers are + * not captured (see the module-level limits). */ typedef struct HookContext { /** @@ -120,6 +158,14 @@ typedef struct HookContext { uint32_t edx; uint32_t ecx; uint32_t eax; + /** + * The SSE control/status register (`MXCSR`). + */ + uint32_t mxcsr; + /** + * `XMM0` through `XMM7`, in register-number order. + */ + struct Xmm xmm[8]; } HookContext; #endif @@ -127,9 +173,10 @@ typedef struct HookContext { * A handler invoked at a mid-function hook site with a pointer to the captured * [`HookContext`]. * - * The handler runs with every register snapshotted and restored around it, so - * it may freely clobber registers and modify the context block in place. It - * must not block indefinitely or unwind across the FFI boundary. + * The handler runs with every general-purpose register, the flags, all XMM + * registers and `MXCSR` snapshotted and restored around it, so it may freely + * clobber registers and modify the context block in place. It must not block + * indefinitely or unwind across the FFI boundary. */ typedef void (*MidHookHandler)(struct HookContext *context); diff --git a/include/neohook.hpp b/include/neohook.hpp index b7f1210..8b5680c 100644 --- a/include/neohook.hpp +++ b/include/neohook.hpp @@ -634,8 +634,9 @@ namespace neohook * * Redirects @p target - which may be any instruction boundary, not just a * function entry - to a context @p handler. The handler receives a pointer - * to a ::HookContext snapshot of the general-purpose registers and flags, - * which it may read or modify before the original instructions resume. + * to a ::HookContext snapshot of the general-purpose registers, flags, XMM + * registers and MXCSR, which it may read or modify before the original + * instructions resume. * * The original bytes are restored when the guard is destroyed. */ diff --git a/src/lib.rs b/src/lib.rs index 71b5bec..1da6755 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -44,7 +44,7 @@ pub use crate::introspect::{ ExportInfo, ImportInfo, ModuleInfo, enumerate_exports, enumerate_imports, enumerate_modules, get_entry_point, }; -pub use crate::midhook::{HookContext, MidHook, MidHookHandler}; +pub use crate::midhook::{HookContext, MidHook, MidHookHandler, Xmm}; pub use crate::module::{ find_function, find_function_by_ordinal, get_module_handle, get_module_size, }; diff --git a/src/midhook.rs b/src/midhook.rs index 917784a..3acadd5 100644 --- a/src/midhook.rs +++ b/src/midhook.rs @@ -20,18 +20,22 @@ //! target (mid-function): JMP stub ; stolen_len bytes, NOP-padded //! //! stub: -//! push all GPRs + flags ; snapshot the live CPU state +//! save XMM regs + MXCSR ; snapshot the SSE / floating-point state +//! push all GPRs + flags ; snapshot the live integer CPU state //! handler(&mut HookContext) ; your code reads / modifies the snapshot -//! pop all GPRs + flags ; apply any edits back to the registers +//! pop all GPRs + flags ; apply any integer edits back +//! restore XMM regs + MXCSR ; apply any SSE / FP edits back //! ; run the original instructions //! JMP target + stolen_len ; resume the function //! ``` //! //! Your handler receives a pointer to a [`HookContext`] mirroring the saved -//! register block. Reads observe the live values; writes are restored into the -//! real registers before the original instructions resume - so a handler can -//! rewrite arguments, results, loop counters, or flags in flight, **without** -//! the function ever returning to a caller. +//! register block - general-purpose registers, the flags register, every XMM +//! register, and the `MXCSR` control/status word. Reads observe the live +//! values; writes are restored into the real registers before the original +//! instructions resume - so a handler can rewrite integer or floating-point / +//! SIMD arguments, results, loop counters, or flags in flight, **without** the +//! function ever returning to a caller. //! //! The patch itself reuses the full inline-hook engine //! ([`crate::transaction`]): threads are suspended, any instruction pointer or @@ -44,8 +48,14 @@ //! - The detour always *continues* the original function; there is no facility //! to skip it or redirect control flow from the handler (modifying the saved //! instruction pointer is intentionally not supported). -//! - Only general-purpose registers and the flags register are captured. XMM / -//! floating-point state is **not** snapshotted; a handler must not disturb it. +//! - General-purpose registers, the flags register, all XMM registers and the +//! `MXCSR` word are captured and restored. The legacy **x87** stack registers +//! (`st0`-`st7`) and MMX state are **not** snapshotted; if your handler runs +//! x87 floating-point code at a site where the interrupted routine has live +//! x87 state, that state may be disturbed. Modern code passes floating-point +//! and SIMD values in XMM registers, which are fully covered. +//! - SSE support is assumed (guaranteed on x86_64, and universal on any x86 CPU +//! running a supported Windows version). //! - `target` must sit on a real instruction boundary. Patching mid-instruction //! corrupts the function. @@ -53,19 +63,40 @@ use crate::DetourError; use crate::alloc::{Trampoline, TrampolineAlloc}; use crate::transaction::{Hook, TransactionCore}; -/// Capacity reserved for the generated context-bridge stub. The real stub is -/// well under 100 bytes on both architectures; this leaves generous headroom. -const STUB_CAPACITY: usize = 256; +/// Capacity reserved for the generated context-bridge stub. With the XMM / +/// MXCSR save-restore block the real stub is ~380 bytes on x86_64 and ~150 on +/// x86; this leaves generous headroom. +const STUB_CAPACITY: usize = 512; -/// Snapshot of the general-purpose registers and flags at the hook site, -/// captured for an x86_64 [`MidHook`] handler. +/// A 128-bit XMM register, captured for a [`MidHook`] handler. /// -/// The field order matches the order in which the stub pushes registers, so the +/// The two halves are stored little-endian, matching how `movups` writes the +/// register to memory: `low` is bytes 0..8 (a packed `f64`, the scalar `f32`, +/// or the low quadword of a vector) and `high` is bytes 8..16. A handler reads +/// or writes these directly - e.g. `f64::from_bits(ctx.xmm[0].low)` to read a +/// scalar `double` argument, or `ctx.xmm[0].low = v.to_bits()` to replace it. +#[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct Xmm { + /// Bytes 0..8 of the register (the low quadword). + pub low: u64, + /// Bytes 8..16 of the register (the high quadword). + pub high: u64, +} + +/// Snapshot of the general-purpose registers, flags and SSE / floating-point +/// state at the hook site, captured for an x86_64 [`MidHook`] handler. +/// +/// The field order matches the order in which the stub saves the state, so the /// pointer passed to the handler aliases this layout exactly. A handler may read /// any field to observe the live value, or write any field to change the /// register before the original instructions resume. `rsp` is captured for /// inspection but writing it has no effect (the stack pointer is managed by the /// stub). +/// +/// `xmm` holds `XMM0`..`XMM15` in order, and `mxcsr` holds the SSE +/// control/status word. The x87 stack registers are not captured (see the +/// module-level limits). #[cfg(target_arch = "x86_64")] #[repr(C)] #[derive(Clone, Copy, Debug)] @@ -87,13 +118,21 @@ pub struct HookContext { pub r13: u64, pub r14: u64, pub r15: u64, + /// The SSE control/status register (`MXCSR`). + pub mxcsr: u32, + /// Padding so `xmm` is 8-byte aligned; mirrors a reserved slot in the stub. + pub _reserved: u32, + /// `XMM0` through `XMM15`, in register-number order. + pub xmm: [Xmm; 16], } -/// Snapshot of the general-purpose registers and flags at the hook site, -/// captured for an x86 [`MidHook`] handler. +/// Snapshot of the general-purpose registers, flags and SSE / floating-point +/// state at the hook site, captured for an x86 [`MidHook`] handler. /// -/// The layout matches the `pushad` + `pushfd` block the stub writes. Writing -/// `esp` has no effect (`popad` discards its saved stack pointer slot). +/// The layout matches the `pushad` + `pushfd` block the stub writes, preceded +/// by the saved `MXCSR` and `XMM0`..`XMM7`. Writing `esp` has no effect +/// (`popad` discards its saved stack pointer slot). The x87 stack registers are +/// not captured (see the module-level limits). #[cfg(target_arch = "x86")] #[repr(C)] #[derive(Clone, Copy, Debug)] @@ -108,14 +147,19 @@ pub struct HookContext { pub edx: u32, pub ecx: u32, pub eax: u32, + /// The SSE control/status register (`MXCSR`). + pub mxcsr: u32, + /// `XMM0` through `XMM7`, in register-number order. + pub xmm: [Xmm; 8], } /// A handler invoked at a mid-function hook site with a pointer to the captured /// [`HookContext`]. /// -/// The handler runs with every register snapshotted and restored around it, so -/// it may freely clobber registers and modify the context block in place. It -/// must not block indefinitely or unwind across the FFI boundary. +/// The handler runs with every general-purpose register, the flags, all XMM +/// registers and `MXCSR` snapshotted and restored around it, so it may freely +/// clobber registers and modify the context block in place. It must not block +/// indefinitely or unwind across the FFI boundary. pub type MidHookHandler = unsafe extern "system" fn(context: *mut HookContext); /// An installed mid-function detour. @@ -232,17 +276,46 @@ impl MidHook { // thread holding the hook; mirror the rest of the crate by not auto-deriving // Send/Sync (callers move the guard into their own synchronization). -/// Emits the context-bridge stub: snapshot all GPRs + flags, call `handler` -/// with a pointer to that block, restore the (possibly modified) registers, then -/// jump to `gateway` (which runs the relocated stolen bytes and resumes the -/// function). `stub_addr` is the address the bytes will live at. +/// Emits the context-bridge stub: snapshot the SSE state (XMM + MXCSR) and all +/// GPRs + flags, call `handler` with a pointer to that block, restore the +/// (possibly modified) state, then jump to `gateway` (which runs the relocated +/// stolen bytes and resumes the function). `stub_addr` is the address the bytes +/// will live at. +/// +/// The combined block, from the lowest address (where the handler pointer aims) +/// upward, is `rflags, rax..r15, mxcsr, _pad, xmm0..xmm15` - exactly the +/// [`HookContext`] layout. The save order is therefore "highest field first": +/// XMM (with `xmm15` pushed first so `xmm0` ends up lowest), then MXCSR, then +/// the GPRs, then flags last. `movups` is used so the stack need not be +/// 16-byte aligned for the spill. #[cfg(target_arch = "x86_64")] fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec { let _ = stub_addr; // x64 stub is position-independent. - let mut c = Vec::with_capacity(96); + let mut c = Vec::with_capacity(STUB_CAPACITY); - // --- save: push r15..r8, rdi, rsi, rbp, rbx, rdx, rcx, rax, then flags --- - // Pushing flags last places it at the lowest address, matching HookContext. + // sub rsp,16 ; movups [rsp], xmmN (REX.R for xmm8..15). + let save_xmm = |c: &mut Vec, n: u8| { + c.extend_from_slice(&[0x48, 0x83, 0xEC, 0x10]); // sub rsp, 16 + if n >= 8 { + c.push(0x44); // REX.R + } + c.extend_from_slice(&[0x0F, 0x11, 0x04 | ((n & 7) << 3), 0x24]); // movups [rsp], xmmN + }; + // movups xmmN, [rsp] ; add rsp,16 + let restore_xmm = |c: &mut Vec, n: u8| { + if n >= 8 { + c.push(0x44); // REX.R + } + c.extend_from_slice(&[0x0F, 0x10, 0x04 | ((n & 7) << 3), 0x24]); // movups xmmN, [rsp] + c.extend_from_slice(&[0x48, 0x83, 0xC4, 0x10]); // add rsp, 16 + }; + + // --- save: xmm15..xmm0 (xmm0 ends lowest), MXCSR, GPRs, flags last --- + for n in (0..16u8).rev() { + save_xmm(&mut c, n); + } + c.extend_from_slice(&[0x48, 0x83, 0xEC, 0x08]); // sub rsp, 8 (mxcsr + 4 pad bytes) + c.extend_from_slice(&[0x0F, 0xAE, 0x1C, 0x24]); // stmxcsr [rsp] c.extend_from_slice(&[ 0x41, 0x57, // push r15 0x41, 0x56, // push r14 @@ -272,7 +345,7 @@ fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec Vec --- clobbers no register. c.extend_from_slice(&[0xFF, 0x25, 0x00, 0x00, 0x00, 0x00]); @@ -301,17 +379,43 @@ fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec Vec { - let mut c = Vec::with_capacity(32); - + let mut c = Vec::with_capacity(STUB_CAPACITY); + + // sub esp,16 ; movups [esp], xmmN + let save_xmm = |c: &mut Vec, n: u8| { + c.extend_from_slice(&[0x83, 0xEC, 0x10]); // sub esp, 16 + c.extend_from_slice(&[0x0F, 0x11, 0x04 | (n << 3), 0x24]); // movups [esp], xmmN + }; + // movups xmmN, [esp] ; add esp,16 + let restore_xmm = |c: &mut Vec, n: u8| { + c.extend_from_slice(&[0x0F, 0x10, 0x04 | (n << 3), 0x24]); // movups xmmN, [esp] + c.extend_from_slice(&[0x83, 0xC4, 0x10]); // add esp, 16 + }; + + // --- save: xmm7..xmm0 (xmm0 ends lowest), MXCSR, then pushad + pushfd --- + for n in (0..8u8).rev() { + save_xmm(&mut c, n); + } + c.extend_from_slice(&[0x83, 0xEC, 0x04]); // sub esp, 4 (mxcsr) + c.extend_from_slice(&[0x0F, 0xAE, 0x1C, 0x24]); // stmxcsr [esp] c.push(0x60); // pushad c.push(0x9C); // pushfd + + // --- call handler(&context) --- c.extend_from_slice(&[0x89, 0xE0]); // mov eax, esp (eax = &context) c.push(0x50); // push eax (arg) c.push(0xB8); // mov eax, imm32 c.extend_from_slice(&(handler as u32).to_le_bytes()); c.extend_from_slice(&[0xFF, 0xD0]); // call eax (stdcall: callee pops the arg) + + // --- restore: flags, GPRs, MXCSR, xmm0..xmm7 --- c.push(0x9D); // popfd c.push(0x61); // popad + c.extend_from_slice(&[0x0F, 0xAE, 0x14, 0x24]); // ldmxcsr [esp] + c.extend_from_slice(&[0x83, 0xC4, 0x04]); // add esp, 4 + for n in 0..8u8 { + restore_xmm(&mut c, n); + } // jmp rel32 gateway (clobbers no register; x86 trampolines are always in range) c.push(0xE9); @@ -345,7 +449,8 @@ mod tests { let gateway = 0x99AA_BBCC_DDEE_FF00u64; let bytes = build_stub(0x4000 as *mut u8, handler as *const u8, gateway as *mut u8); - // The handler appears as an imm64 after `mov rax, imm64` (48 B8). + // The handler appears as an imm64 after `mov rax, imm64` (48 B8). The + // XMM spill prologue never emits that pair, so the first hit is the call. let movabs = bytes .windows(2) .position(|w| w == [0x48, 0xB8]) @@ -357,6 +462,54 @@ mod tests { let tail = u64::from_le_bytes(bytes[bytes.len() - 8..].try_into().unwrap()); assert_eq!(tail, gateway); assert_eq!(&bytes[bytes.len() - 14..bytes.len() - 8], &[0xFF, 0x25, 0, 0, 0, 0]); + + // The SSE state is saved (stmxcsr 0F AE 1C 24) and restored + // (ldmxcsr 0F AE 14 24) exactly once around the call. + assert_eq!( + bytes.windows(4).filter(|w| *w == [0x0F, 0xAE, 0x1C, 0x24]).count(), + 1, + "exactly one stmxcsr" + ); + assert_eq!( + bytes.windows(4).filter(|w| *w == [0x0F, 0xAE, 0x14, 0x24]).count(), + 1, + "exactly one ldmxcsr" + ); + // 16 XMM spills (movups store, 0F 11) and 16 reloads (movups load, 0F 10). + assert_eq!( + bytes.windows(2).filter(|w| *w == [0x0F, 0x11]).count(), + 16, + "one movups store per XMM register" + ); + assert_eq!( + bytes.windows(2).filter(|w| *w == [0x0F, 0x10]).count(), + 16, + "one movups load per XMM register" + ); + } + + #[cfg(target_arch = "x86_64")] + #[test] + fn x64_context_layout_matches_stub() { + use std::mem::{offset_of, size_of}; + assert_eq!(size_of::(), 16); + assert_eq!(offset_of!(HookContext, rflags), 0); + assert_eq!(offset_of!(HookContext, r15), 120); + assert_eq!(offset_of!(HookContext, mxcsr), 128); + assert_eq!(offset_of!(HookContext, xmm), 136); + assert_eq!(size_of::(), 136 + 16 * 16); + } + + #[cfg(target_arch = "x86")] + #[test] + fn x86_context_layout_matches_stub() { + use std::mem::{offset_of, size_of}; + assert_eq!(size_of::(), 16); + assert_eq!(offset_of!(HookContext, eflags), 0); + assert_eq!(offset_of!(HookContext, eax), 32); + assert_eq!(offset_of!(HookContext, mxcsr), 36); + assert_eq!(offset_of!(HookContext, xmm), 40); + assert_eq!(size_of::(), 40 + 8 * 16); } #[cfg(target_arch = "x86")] @@ -366,7 +519,10 @@ mod tests { let gateway = 0x0050_0000i64; let bytes = build_stub(stub_addr as *mut u8, 0xCAFE as *const u8, gateway as *mut u8); - assert_eq!(bytes[0], 0x60, "pushad first"); + // The XMM spill prologue now runs before pushad; the first thing the + // stub does is reserve a 16-byte slot (sub esp, 16) for xmm7. + assert_eq!(&bytes[0..3], &[0x83, 0xEC, 0x10], "first op is sub esp, 16"); + assert!(bytes.contains(&0x60), "pushad present"); assert_eq!(*bytes.last_chunk::<5>().unwrap().first().unwrap(), 0xE9); let rel = i32::from_le_bytes(bytes[bytes.len() - 4..].try_into().unwrap()) as i64; diff --git a/tests/midhook.rs b/tests/midhook.rs index f86c78c..01c3788 100644 --- a/tests/midhook.rs +++ b/tests/midhook.rs @@ -9,9 +9,21 @@ //! "called" and "unhook restores" tests on every target. use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::Mutex; use neohook::{HookContext, MidHook}; +// Installing a MidHook suspends every other thread for the duration of the +// patch. The default test runner executes these tests in parallel, so two +// installs racing on the global suspend/relocate machinery (often on the same +// target function) can collide. Serialize the install/unhook sections so the +// suite is deterministic regardless of `--test-threads`. +static INSTALL_LOCK: Mutex<()> = Mutex::new(()); + +fn install_guard() -> std::sync::MutexGuard<'static, ()> { + INSTALL_LOCK.lock().unwrap_or_else(|e| e.into_inner()) +} + // A function whose entry we treat as an arbitrary code address to detour. // `black_box` keeps the compiler from constant-folding or inlining it away. #[inline(never)] @@ -43,8 +55,40 @@ unsafe extern "system" fn add_to_arg_handler(ctx: *mut HookContext) { ctx.rcx = ctx.rcx.wrapping_add(10); } +// On Win64 the first floating-point argument arrives in XMM0, so a mid-hook at +// the entry of these functions can observe / rewrite it through `ctx.xmm[0]`. +#[cfg(target_arch = "x86_64")] +#[inline(never)] +extern "system" fn scale(x: f64) -> f64 { + std::hint::black_box(x) * 2.0 +} + +#[cfg(target_arch = "x86_64")] +#[inline(never)] +extern "system" fn add_half(x: f64) -> f64 { + std::hint::black_box(x) + 0.5 +} + +#[cfg(target_arch = "x86_64")] +static OBSERVED_XMM: AtomicU64 = AtomicU64::new(0); + +#[cfg(target_arch = "x86_64")] +unsafe extern "system" fn observe_xmm_handler(ctx: *mut HookContext) { + let ctx = unsafe { &*ctx }; + OBSERVED_XMM.store(ctx.xmm[0].low, Ordering::SeqCst); // low 64 bits = the f64 +} + +#[cfg(target_arch = "x86_64")] +unsafe extern "system" fn bump_xmm0_handler(ctx: *mut HookContext) { + // Rewrite the first floating-point argument (XMM0) in flight: x -> x + 100. + let ctx = unsafe { &mut *ctx }; + let v = f64::from_bits(ctx.xmm[0].low); + ctx.xmm[0].low = (v + 100.0).to_bits(); +} + #[test] fn handler_runs_and_function_still_returns_correctly() { + let _serial = install_guard(); HANDLER_RAN.store(false, Ordering::SeqCst); assert_eq!(noop_work(41), 42); // baseline @@ -71,6 +115,7 @@ fn handler_runs_and_function_still_returns_correctly() { #[cfg(target_arch = "x86_64")] #[test] fn handler_observes_live_argument_register() { + let _serial = install_guard(); OBSERVED_ARG.store(0, Ordering::SeqCst); let hook = unsafe { MidHook::install(triple as *const u8, observe_handler) } @@ -89,6 +134,7 @@ fn handler_observes_live_argument_register() { #[cfg(target_arch = "x86_64")] #[test] fn handler_can_rewrite_a_register_in_flight() { + let _serial = install_guard(); assert_eq!(triple(5), 15); // baseline: 5 * 3 let hook = unsafe { MidHook::install(triple as *const u8, add_to_arg_handler) } @@ -101,6 +147,42 @@ fn handler_can_rewrite_a_register_in_flight() { assert_eq!(triple(5), 15, "behaviour restored after unhook"); } +#[cfg(target_arch = "x86_64")] +#[test] +fn handler_observes_live_xmm_argument() { + let _serial = install_guard(); + OBSERVED_XMM.store(0, Ordering::SeqCst); + + let hook = + unsafe { MidHook::install(scale as *const u8, observe_xmm_handler) }.expect("install"); + + let _ = scale(3.5); + assert_eq!( + f64::from_bits(OBSERVED_XMM.load(Ordering::SeqCst)), + 3.5, + "handler should see the f64 argument in XMM0" + ); + + hook.unhook().expect("unhook"); +} + +#[cfg(target_arch = "x86_64")] +#[test] +fn handler_can_rewrite_an_xmm_register_in_flight() { + let _serial = install_guard(); + assert_eq!(add_half(1.0), 1.5); // baseline: 1.0 + 0.5 + + let hook = + unsafe { MidHook::install(add_half as *const u8, bump_xmm0_handler) }.expect("install"); + + // Handler bumps XMM0 (the argument) by 100 before the body runs: + // (1.0 + 100.0) + 0.5. + assert_eq!(add_half(1.0), 101.5, "XMM register edit must take effect"); + + hook.unhook().expect("unhook"); + assert_eq!(add_half(1.0), 1.5, "behaviour restored after unhook"); +} + #[test] fn install_rejects_null_target() { let err = unsafe { MidHook::install(std::ptr::null(), observe_handler) }; @@ -109,6 +191,7 @@ fn install_rejects_null_target() { #[test] fn dropping_the_guard_unhooks() { + let _serial = install_guard(); HANDLER_RAN.store(false, Ordering::SeqCst); { From a942fe376b97f968f357d56f9869c9f41610e2a5 Mon Sep 17 00:00:00 2001 From: AlienDwarf <61460778+AlienDwarf@users.noreply.github.com> Date: Sat, 20 Jun 2026 02:24:10 +0200 Subject: [PATCH 2/5] style: format the codebase with rustfmt The repository had accumulated rustfmt drift across many modules, examples and tests, which fails the `cargo fmt --all --check` step in CI. Run rustfmt over the whole tree so the formatting check passes. No functional changes. --- examples/attach_export.rs | 19 ++++++++++++------- examples/closure_detour.rs | 17 +++++++++++++---- examples/d3d11_present.rs | 32 ++++++++++++++++++-------------- examples/d3d9_endscene.rs | 25 +++++++++++++++++++------ examples/delay_hook.rs | 11 ++++++----- examples/eat_hook.rs | 3 ++- examples/opengl_swapbuffers.rs | 6 +++--- examples/pattern_scan.rs | 4 ++-- src/api.rs | 4 ++-- src/delay.rs | 18 +++++++++--------- src/eat.rs | 8 +++++--- src/introspect.rs | 9 +++++---- src/lib.rs | 8 ++++++-- src/midhook.rs | 27 ++++++++++++++++++++++----- src/module.rs | 5 +---- src/registry.rs | 7 +++++-- src/scan.rs | 5 ++++- src/transaction.rs | 6 +----- src/veh.rs | 5 +++-- tests/eat_hooks.rs | 34 +++++++++++++++++++++++++++------- tests/introspect.rs | 4 +--- tests/midhook.rs | 18 ++++++++++-------- tests/pattern_scan.rs | 12 ++++++++---- tests/veh_hooks.rs | 20 ++++++++++++++------ 24 files changed, 198 insertions(+), 109 deletions(-) diff --git a/examples/attach_export.rs b/examples/attach_export.rs index 4641c3d..fcd12c1 100644 --- a/examples/attach_export.rs +++ b/examples/attach_export.rs @@ -24,7 +24,9 @@ unsafe extern "system" fn hooked_get_tick_count() -> u32 { } fn main() -> Result<(), Box> { - println!("before hook: GetTickCount() = {}", unsafe { GetTickCount() }); + println!("before hook: GetTickCount() = {}", unsafe { + GetTickCount() + }); let mut tx = DetourTransaction::begin(); tx.update_all_threads(); @@ -39,15 +41,18 @@ fn main() -> Result<(), Box> { let _hooks = tx.commit()?; let _ = ORIG.set(unsafe { std::mem::transmute::<*mut u8, GetTickCountFn>(tramp) }); - println!("after hook: GetTickCount() = {}", unsafe { GetTickCount() }); - println!( - "original via trampoline: = {}", - unsafe { (ORIG.get().unwrap())() } - ); + println!("after hook: GetTickCount() = {}", unsafe { + GetTickCount() + }); + println!("original via trampoline: = {}", unsafe { + (ORIG.get().unwrap())() + }); // _hooks drops here -> the original bytes are restored automatically. drop(_hooks); - println!("after unhook:GetTickCount() = {}", unsafe { GetTickCount() }); + println!("after unhook:GetTickCount() = {}", unsafe { + GetTickCount() + }); Ok(()) } diff --git a/examples/closure_detour.rs b/examples/closure_detour.rs index 5bdfcb4..ecc3d6d 100644 --- a/examples/closure_detour.rs +++ b/examples/closure_detour.rs @@ -11,8 +11,8 @@ */ use neohook::detour_closure; use std::error::Error; -use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; +use std::sync::atomic::{AtomicU32, Ordering}; #[inline(never)] extern "system" fn add(a: i32, b: i32) -> i32 { @@ -40,9 +40,18 @@ fn main() -> Result<(), Box> { }, )?; - println!("after hook: add(2, 3) = {} (intercepted, call #1)", call(2, 3)); - println!("after hook: add(4, 5) = {} (intercepted, call #2)", call(4, 5)); - println!("total intercepted calls: {}", counter.load(Ordering::Relaxed)); + println!( + "after hook: add(2, 3) = {} (intercepted, call #1)", + call(2, 3) + ); + println!( + "after hook: add(4, 5) = {} (intercepted, call #2)", + call(4, 5) + ); + println!( + "total intercepted calls: {}", + counter.load(Ordering::Relaxed) + ); drop(hooks); // RAII restores the original bytes. println!("after unhook: add(2, 3) = {}", call(2, 3)); diff --git a/examples/d3d11_present.rs b/examples/d3d11_present.rs index c6f1526..328d5d2 100644 --- a/examples/d3d11_present.rs +++ b/examples/d3d11_present.rs @@ -11,9 +11,9 @@ use std::sync::atomic::{AtomicPtr, Ordering}; use windows_sys::Win32::Foundation::HWND; use windows_sys::Win32::System::LibraryLoader::{GetModuleHandleW, GetProcAddress, LoadLibraryW}; use windows_sys::Win32::UI::WindowsAndMessaging::{ - CreateWindowExW, DefWindowProcW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, - PeekMessageW, PostQuitMessage, RegisterClassW, SW_SHOW, ShowWindow, TranslateMessage, - WM_DESTROY, WM_QUIT, WNDCLASSW, WS_OVERLAPPEDWINDOW, + CreateWindowExW, DefWindowProcW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, PeekMessageW, + PostQuitMessage, RegisterClassW, SW_SHOW, ShowWindow, TranslateMessage, WM_DESTROY, WM_QUIT, + WNDCLASSW, WS_OVERLAPPEDWINDOW, }; use windows_sys::core::GUID; @@ -109,9 +109,11 @@ type D3d11CreateFn = unsafe extern "system" fn( /// `HRESULT IDXGISwapChain::Present(this, SyncInterval, Flags)`. type PresentFn = unsafe extern "system" fn(*mut c_void, u32, u32) -> i32; /// `HRESULT GetBuffer(this, Buffer, riid, ppSurface)`. -type GetBufferFn = unsafe extern "system" fn(*mut c_void, u32, *const GUID, *mut *mut c_void) -> i32; +type GetBufferFn = + unsafe extern "system" fn(*mut c_void, u32, *const GUID, *mut *mut c_void) -> i32; /// `HRESULT QueryInterface(this, riid, ppvObject)`. -type QueryInterfaceFn = unsafe extern "system" fn(*mut c_void, *const GUID, *mut *mut c_void) -> i32; +type QueryInterfaceFn = + unsafe extern "system" fn(*mut c_void, *const GUID, *mut *mut c_void) -> i32; /// `HRESULT CreateRenderTargetView(this, pResource, pDesc, ppRTV)`. type CreateRtvFn = unsafe extern "system" fn(*mut c_void, *mut c_void, *const c_void, *mut *mut c_void) -> i32; @@ -184,14 +186,16 @@ fn main() -> Result<(), Box> { println!("skipped: d3d11.dll not available"); return Ok(()); } - let create_ptr = - match GetProcAddress(d3d11, c"D3D11CreateDeviceAndSwapChain".as_ptr() as *const u8) { - Some(p) => p, - None => { - println!("skipped: D3D11CreateDeviceAndSwapChain not found"); - return Ok(()); - } - }; + let create_ptr = match GetProcAddress( + d3d11, + c"D3D11CreateDeviceAndSwapChain".as_ptr() as *const u8, + ) { + Some(p) => p, + None => { + println!("skipped: D3D11CreateDeviceAndSwapChain not found"); + return Ok(()); + } + }; let create: D3d11CreateFn = std::mem::transmute(create_ptr); //Create the window @@ -308,7 +312,7 @@ fn main() -> Result<(), Box> { RTV.store(rtv, Ordering::Relaxed); CTX1.store(context1, Ordering::Relaxed); - // Hook Present by cloning the swapchain's vtable, replacing the Present slot, and writing a + // Hook Present by cloning the swapchain's vtable, replacing the Present slot, and writing a // pointer to the clone back into the swapchain. let mut tx = DetourTransaction::begin(); let original = tx.attach_vtable_instance( diff --git a/examples/d3d9_endscene.rs b/examples/d3d9_endscene.rs index dbf7606..e6632f2 100644 --- a/examples/d3d9_endscene.rs +++ b/examples/d3d9_endscene.rs @@ -19,9 +19,9 @@ use std::sync::OnceLock; use windows_sys::Win32::Foundation::HWND; use windows_sys::Win32::System::LibraryLoader::{GetModuleHandleW, GetProcAddress, LoadLibraryW}; use windows_sys::Win32::UI::WindowsAndMessaging::{ - CreateWindowExW, DefWindowProcW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, - PeekMessageW, PostQuitMessage, RegisterClassW, SW_SHOW, ShowWindow, TranslateMessage, - WM_DESTROY, WM_QUIT, WNDCLASSW, WS_OVERLAPPEDWINDOW, + CreateWindowExW, DefWindowProcW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, PeekMessageW, + PostQuitMessage, RegisterClassW, SW_SHOW, ShowWindow, TranslateMessage, WM_DESTROY, WM_QUIT, + WNDCLASSW, WS_OVERLAPPEDWINDOW, }; // --- Minimal Direct3D 9 definitions (not provided by windows-sys) --- @@ -90,8 +90,13 @@ type SceneFn = unsafe extern "system" fn(*mut c_void) -> i32; type ClearFn = unsafe extern "system" fn(*mut c_void, u32, *const D3dRect, u32, u32, f32, u32) -> i32; /// `HRESULT Present(this, pSrcRect, pDstRect, hDestWindow, pDirtyRegion)`. -type PresentFn = - unsafe extern "system" fn(*mut c_void, *const c_void, *const c_void, HWND, *const c_void) -> i32; +type PresentFn = unsafe extern "system" fn( + *mut c_void, + *const c_void, + *const c_void, + HWND, + *const c_void, +) -> i32; type ReleaseFn = unsafe extern "system" fn(*mut c_void) -> u32; /// The original `EndScene` returned by `attach_vtable`, used to finalize frames. @@ -265,7 +270,15 @@ fn main() -> Result<(), Box> { TranslateMessage(&msg); DispatchMessageW(&msg); } - clear(device, 0, std::ptr::null(), D3DCLEAR_TARGET, 0xFF00_0000, 1.0, 0); + clear( + device, + 0, + std::ptr::null(), + D3DCLEAR_TARGET, + 0xFF00_0000, + 1.0, + 0, + ); begin_scene(device); // EndScene dispatches through the hooked vtable slot. let end_scene: SceneFn = std::mem::transmute(*vtable.add(END_SCENE_SLOT)); diff --git a/examples/delay_hook.rs b/examples/delay_hook.rs index e8823e0..fe61d59 100644 --- a/examples/delay_hook.rs +++ b/examples/delay_hook.rs @@ -40,13 +40,14 @@ fn main() -> Result<(), Box> { let proc = unsafe { GetProcAddress(module, c"timeGetTime".as_ptr() as *const u8) } .ok_or("timeGetTime not found")?; let time_get_time: unsafe extern "system" fn() -> u32 = unsafe { std::mem::transmute(proc) }; - println!( - "timeGetTime() = {} (0xDEADBEE1 - intercepted)", - unsafe { time_get_time() } - ); + println!("timeGetTime() = {} (0xDEADBEE1 - intercepted)", unsafe { + time_get_time() + }); hook.unhook()?; - println!("after unhook: timeGetTime() = {}", unsafe { time_get_time() }); + println!("after unhook: timeGetTime() = {}", unsafe { + time_get_time() + }); Ok(()) } diff --git a/examples/eat_hook.rs b/examples/eat_hook.rs index 061cc00..c8134a2 100644 --- a/examples/eat_hook.rs +++ b/examples/eat_hook.rs @@ -60,7 +60,8 @@ fn main() -> Result<(), Box> { // The hook records the original resolved address; calling it bypasses the // detour and reaches the real function body. - let original = unsafe { std::mem::transmute::<*const u8, GetTickCountFn>(hooks[0].original_ptr()) }; + let original = + unsafe { std::mem::transmute::<*const u8, GetTickCountFn>(hooks[0].original_ptr()) }; println!("original still returns ~{}", unsafe { original() }); for hook in hooks { diff --git a/examples/opengl_swapbuffers.rs b/examples/opengl_swapbuffers.rs index 11f9b03..fc46db7 100644 --- a/examples/opengl_swapbuffers.rs +++ b/examples/opengl_swapbuffers.rs @@ -27,9 +27,9 @@ use windows_sys::Win32::Graphics::OpenGL::{ }; use windows_sys::Win32::System::LibraryLoader::{GetModuleHandleW, GetProcAddress}; use windows_sys::Win32::UI::WindowsAndMessaging::{ - CreateWindowExW, DefWindowProcW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, - PeekMessageW, PostQuitMessage, RegisterClassW, SW_SHOW, ShowWindow, TranslateMessage, - WM_DESTROY, WM_QUIT, WNDCLASSW, WS_OVERLAPPEDWINDOW, + CreateWindowExW, DefWindowProcW, DestroyWindow, DispatchMessageW, MSG, PM_REMOVE, PeekMessageW, + PostQuitMessage, RegisterClassW, SW_SHOW, ShowWindow, TranslateMessage, WM_DESTROY, WM_QUIT, + WNDCLASSW, WS_OVERLAPPEDWINDOW, }; const GL_COLOR_BUFFER_BIT: u32 = 0x0000_4000; diff --git a/examples/pattern_scan.rs b/examples/pattern_scan.rs index 8ffad74..f980eb4 100644 --- a/examples/pattern_scan.rs +++ b/examples/pattern_scan.rs @@ -67,8 +67,8 @@ fn main() { } // You can also build a signature from a byte array + mask string: - let _code_style = Pattern::from_code_style(b"\x48\x8B\x00\x00", "xx??") - .expect("valid code-style signature"); + let _code_style = + Pattern::from_code_style(b"\x48\x8B\x00\x00", "xx??").expect("valid code-style signature"); // --- 2. Hook the function purely by signature --------------------------- println!("\ncompute(10) before hook = {}", compute(10)); // 31 diff --git a/src/api.rs b/src/api.rs index cb9c70d..0db4be3 100644 --- a/src/api.rs +++ b/src/api.rs @@ -146,8 +146,8 @@ impl DetourTransaction { function_name: &str, detour: *const u8, ) -> Result<*mut u8, DetourError> { - let target = - crate::find_function(module_name, function_name).ok_or(DetourError::InvalidParameter)?; + let target = crate::find_function(module_name, function_name) + .ok_or(DetourError::InvalidParameter)?; self.inner .as_mut() diff --git a/src/delay.rs b/src/delay.rs index 55c80ea..d9d722a 100644 --- a/src/delay.rs +++ b/src/delay.rs @@ -218,8 +218,8 @@ fn ensure_ldr_hook(mgr: &mut DelayManager) -> Result<(), DelayHookError> { return Ok(()); } - let target = crate::find_function("ntdll.dll", "LdrLoadDll") - .ok_or(DelayHookError::LdrHookFailed)?; + let target = + crate::find_function("ntdll.dll", "LdrLoadDll").ok_or(DelayHookError::LdrHookFailed)?; let mut tx = TransactionCore::begin(); tx.update_all_threads(); @@ -299,8 +299,9 @@ mod tests { // first and loading afterwards exercises the LdrLoadDll path. (If it is // already loaded, the immediate-install path still leaves the hook // active, so the end-state assertions below hold either way.) - let hook = unsafe { DelayHook::register("winmm.dll", "timeGetTime", fake_time as *const u8) } - .expect("register should succeed"); + let hook = + unsafe { DelayHook::register("winmm.dll", "timeGetTime", fake_time as *const u8) } + .expect("register should succeed"); // Force the load; our LdrLoadDll detour installs the pending hook. let module = unsafe { LoadLibraryW(wide("winmm.dll").as_ptr()) }; @@ -312,11 +313,10 @@ mod tests { ); // Resolve and call the real export: the INT3 redirect should win. - let proc = unsafe { - GetProcAddress(module, c"timeGetTime".as_ptr() as *const u8) - } - .expect("timeGetTime should resolve"); - let time_get_time: unsafe extern "system" fn() -> u32 = unsafe { std::mem::transmute(proc) }; + let proc = unsafe { GetProcAddress(module, c"timeGetTime".as_ptr() as *const u8) } + .expect("timeGetTime should resolve"); + let time_get_time: unsafe extern "system" fn() -> u32 = + unsafe { std::mem::transmute(proc) }; assert_eq!( unsafe { time_get_time() }, 0xABCD_1234, diff --git a/src/eat.rs b/src/eat.rs index 727f1a0..53b24f9 100644 --- a/src/eat.rs +++ b/src/eat.rs @@ -245,9 +245,11 @@ unsafe fn find_export_slot( } let slot_rva = functions_rva - .checked_add(func_index.checked_mul(std::mem::size_of::()).ok_or( - EatHookError::ExportTableUnavailable, - )?) + .checked_add( + func_index + .checked_mul(std::mem::size_of::()) + .ok_or(EatHookError::ExportTableUnavailable)?, + ) .ok_or(EatHookError::ExportTableUnavailable)?; let slot_ptr = pe::rva_to_mut_ptr::(image, slot_rva) diff --git a/src/introspect.rs b/src/introspect.rs index b5951a2..c5a5fcc 100644 --- a/src/introspect.rs +++ b/src/introspect.rs @@ -68,8 +68,7 @@ pub fn enumerate_modules() -> Vec { // Snapshot the modules of the current process (pid 0 == self). Both the // native and 32-bit module lists are requested so a WoW64 process sees all // of its modules. - let snapshot = - unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, 0) }; + let snapshot = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPMODULE | TH32CS_SNAPMODULE32, 0) }; if snapshot == INVALID_HANDLE_VALUE { return modules; } @@ -131,7 +130,8 @@ pub unsafe fn enumerate_exports(h_module: HMODULE) -> Result, Pe let mut exports = Vec::new(); // No export directory => the module has no exports. - let Some((exp_rva, exp_size)) = pe::data_directory(&image, IMAGE_DIRECTORY_ENTRY_EXPORT as usize) + let Some((exp_rva, exp_size)) = + pe::data_directory(&image, IMAGE_DIRECTORY_ENTRY_EXPORT as usize) else { return Ok(exports); }; @@ -260,7 +260,8 @@ pub unsafe fn enumerate_imports(h_module: HMODULE) -> Result, Pe } else { // `lookup` is the RVA of an IMAGE_IMPORT_BY_NAME (WORD hint // followed by the NUL-terminated name). - let name = pe::read_c_string_from_rva(&image, lookup + std::mem::size_of::()); + let name = + pe::read_c_string_from_rva(&image, lookup + std::mem::size_of::()); (name, None) }; diff --git a/src/lib.rs b/src/lib.rs index 1da6755..a034cab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -396,8 +396,8 @@ mod tests { #[test] fn detour_closure_captures_environment_and_forwards_to_original() { - use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; + use std::sync::atomic::{AtomicU32, Ordering}; let calls = Arc::new(AtomicU32::new(0)); let calls_in = Arc::clone(&calls); @@ -418,7 +418,11 @@ mod tests { assert_eq!(target(2, 3), 50, "(2 + 3) * 10 via the closure detour"); assert_eq!(target(4, 5), 90, "(4 + 5) * 10"); - assert_eq!(calls.load(Ordering::Relaxed), 2, "closure captured the counter"); + assert_eq!( + calls.load(Ordering::Relaxed), + 2, + "closure captured the counter" + ); drop(hooks); // RAII restores the original. assert_eq!(target(2, 3), 5, "original restored after unhook"); diff --git a/src/midhook.rs b/src/midhook.rs index 3acadd5..cab84b2 100644 --- a/src/midhook.rs +++ b/src/midhook.rs @@ -433,7 +433,11 @@ mod tests { #[test] fn stub_fits_within_capacity() { - let bytes = build_stub(0x1000 as *mut u8, 0xDEAD_BEEF as *const u8, 0x2000 as *mut u8); + let bytes = build_stub( + 0x1000 as *mut u8, + 0xDEAD_BEEF as *const u8, + 0x2000 as *mut u8, + ); assert!( bytes.len() <= STUB_CAPACITY, "stub ({} bytes) must fit in the reserved buffer", @@ -461,17 +465,26 @@ mod tests { // The gateway is the trailing 8 bytes after the FF 25 absolute jump. let tail = u64::from_le_bytes(bytes[bytes.len() - 8..].try_into().unwrap()); assert_eq!(tail, gateway); - assert_eq!(&bytes[bytes.len() - 14..bytes.len() - 8], &[0xFF, 0x25, 0, 0, 0, 0]); + assert_eq!( + &bytes[bytes.len() - 14..bytes.len() - 8], + &[0xFF, 0x25, 0, 0, 0, 0] + ); // The SSE state is saved (stmxcsr 0F AE 1C 24) and restored // (ldmxcsr 0F AE 14 24) exactly once around the call. assert_eq!( - bytes.windows(4).filter(|w| *w == [0x0F, 0xAE, 0x1C, 0x24]).count(), + bytes + .windows(4) + .filter(|w| *w == [0x0F, 0xAE, 0x1C, 0x24]) + .count(), 1, "exactly one stmxcsr" ); assert_eq!( - bytes.windows(4).filter(|w| *w == [0x0F, 0xAE, 0x14, 0x24]).count(), + bytes + .windows(4) + .filter(|w| *w == [0x0F, 0xAE, 0x14, 0x24]) + .count(), 1, "exactly one ldmxcsr" ); @@ -517,7 +530,11 @@ mod tests { fn x86_stub_encodes_relative_jump_to_gateway() { let stub_addr = 0x0040_0000i64; let gateway = 0x0050_0000i64; - let bytes = build_stub(stub_addr as *mut u8, 0xCAFE as *const u8, gateway as *mut u8); + let bytes = build_stub( + stub_addr as *mut u8, + 0xCAFE as *const u8, + gateway as *mut u8, + ); // The XMM spill prologue now runs before pushad; the first thing the // stub does is reserve a 16-byte slot (sub esp, 16) for xmm7. diff --git a/src/module.rs b/src/module.rs index ffd20bb..651241a 100644 --- a/src/module.rs +++ b/src/module.rs @@ -205,10 +205,7 @@ mod tests { #[test] fn find_function_by_ordinal_returns_none_for_absurd_ordinal() { let addr = module::find_function_by_ordinal("kernel32.dll", u16::MAX); - assert!( - addr.is_none(), - "an out-of-range ordinal should not resolve" - ); + assert!(addr.is_none(), "an out-of-range ordinal should not resolve"); } #[test] diff --git a/src/registry.rs b/src/registry.rs index 899f0f2..c915e1d 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -33,8 +33,8 @@ use std::collections::HashMap; use std::sync::Mutex; -use crate::transaction::Hook; use crate::DetourError; +use crate::transaction::Hook; /// Wraps a [`Hook`] so it can live in a `static`. /// @@ -106,7 +106,10 @@ pub fn disable(name: &str) -> Result { /// registered under `name`, or `None` if no such hook is registered. pub fn is_enabled(name: &str) -> Option { let guard = lock(); - guard.as_ref().and_then(|m| m.get(name)).map(|h| h.0.is_enabled()) + guard + .as_ref() + .and_then(|m| m.get(name)) + .map(|h| h.0.is_enabled()) } /// Removes the hook registered under `name` and unhooks it, restoring the diff --git a/src/scan.rs b/src/scan.rs index 601e7bf..ff59605 100644 --- a/src/scan.rs +++ b/src/scan.rs @@ -64,7 +64,10 @@ impl std::fmt::Display for PatternError { match self { Self::Empty => write!(f, "signature is empty"), Self::InvalidToken(tok) => { - write!(f, "invalid signature token '{tok}' (expected a hex byte or '?')") + write!( + f, + "invalid signature token '{tok}' (expected a hex byte or '?')" + ) } Self::InvalidMaskChar(c) => { write!(f, "invalid mask character '{c}' (expected 'x' or '?')") diff --git a/src/transaction.rs b/src/transaction.rs index cf35b0f..f851473 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -696,11 +696,7 @@ impl TransactionCore { /// # Safety /// The caller must ensure that `target` and `detour` are valid pointers. NeoHook performs basic validation but does not guarantee that the pointers are valid or that the memory they point to is properly aligned or accessible. Invalid pointers may cause undefined behavior, including crashes. #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub fn attach( - &mut self, - target: *mut u8, - detour: *const u8, - ) -> Result<*mut u8, DetourError> { + pub fn attach(&mut self, target: *mut u8, detour: *const u8) -> Result<*mut u8, DetourError> { self.attach_inner(target, detour, true) } diff --git a/src/veh.rs b/src/veh.rs index a89c5f5..e02abac 100644 --- a/src/veh.rs +++ b/src/veh.rs @@ -3,7 +3,7 @@ //! VEH (Vectored Exception Handler) //! -//! A VEH hook does **not** modify a single byte of the target - +//! A VEH hook does **not** modify a single byte of the target - //! neither its code nor any pointer table. Instead //! it uses a CPU hardware execution breakpoint (debug registers `DR0`-`DR3`) on //! the target address and installs a process-wide vectored exception handler. @@ -411,7 +411,8 @@ mod tests { let target = slot_target_a as *const () as *const u8; let detour = slot_detour as *const () as *const u8; - let hook = unsafe { VehHook::install(target, detour) }.expect("first install should succeed"); + let hook = + unsafe { VehHook::install(target, detour) }.expect("first install should succeed"); assert!(matches!( unsafe { VehHook::install(target, detour) }, Err(VehHookError::AlreadyHooked) diff --git a/tests/eat_hooks.rs b/tests/eat_hooks.rs index 909cdc4..7da400d 100644 --- a/tests/eat_hooks.rs +++ b/tests/eat_hooks.rs @@ -38,7 +38,10 @@ fn eat_hook_redirects_getprocaddress_and_restores() { let module = kernel32(); let before = unsafe { resolve(module, "GetTickCount")() }; - assert_ne!(before, SENTINEL, "precondition: real function must not return the sentinel"); + assert_ne!( + before, SENTINEL, + "precondition: real function must not return the sentinel" + ); let mut tx = TransactionCore::begin(); tx.attach_eat(module, "GetTickCount", get_tick_count_detour as *const u8) @@ -47,19 +50,29 @@ fn eat_hook_redirects_getprocaddress_and_restores() { // A fresh resolution now lands on the detour (directly or via the jump stub). let hooked = unsafe { resolve(module, "GetTickCount")() }; - assert_eq!(hooked, SENTINEL, "EAT lookup should hit the detour after commit"); + assert_eq!( + hooked, SENTINEL, + "EAT lookup should hit the detour after commit" + ); // The recorded original pointer still reaches the real function body. let original = unsafe { std::mem::transmute::<*const u8, GetTickCountFn>(hooks[0].original_ptr()) }; - assert_ne!(unsafe { original() }, SENTINEL, "original_ptr must bypass the detour"); + assert_ne!( + unsafe { original() }, + SENTINEL, + "original_ptr must bypass the detour" + ); for hook in hooks { hook.unhook().expect("unhook should succeed"); } let after = unsafe { resolve(module, "GetTickCount")() }; - assert_ne!(after, SENTINEL, "EAT lookup should be restored after unhook"); + assert_ne!( + after, SENTINEL, + "EAT lookup should be restored after unhook" + ); } #[test] @@ -73,7 +86,9 @@ fn eat_hook_disable_enable_round_trip() { assert_eq!(unsafe { resolve(module, "GetTickCount")() }, SENTINEL); - hooks[0].disable().expect("disable should restore the original RVA"); + hooks[0] + .disable() + .expect("disable should restore the original RVA"); assert_ne!( unsafe { resolve(module, "GetTickCount")() }, SENTINEL, @@ -81,7 +96,9 @@ fn eat_hook_disable_enable_round_trip() { ); assert!(!hooks[0].is_enabled()); - hooks[0].enable().expect("enable should re-point at the detour"); + hooks[0] + .enable() + .expect("enable should re-point at the detour"); assert_eq!( unsafe { resolve(module, "GetTickCount")() }, SENTINEL, @@ -104,7 +121,10 @@ fn attach_eat_rejects_missing_export() { "DefinitelyNotARealExport_123", get_tick_count_detour as *const u8, ); - assert!(result.is_err(), "missing export should be rejected at attach time"); + assert!( + result.is_err(), + "missing export should be rejected at attach time" + ); } #[test] diff --git a/tests/introspect.rs b/tests/introspect.rs index 05e1f5f..52e5ff8 100644 --- a/tests/introspect.rs +++ b/tests/introspect.rs @@ -80,9 +80,7 @@ fn find_function_by_ordinal_matches_named_lookup() { let sample = exports .iter() - .find(|e| { - e.name.is_some() && e.forwarder.is_none() && e.ordinal <= u16::MAX as u32 - }) + .find(|e| e.name.is_some() && e.forwarder.is_none() && e.ordinal <= u16::MAX as u32) .expect("kernel32 should have a named, non-forwarded export"); let name = sample.name.as_deref().unwrap(); diff --git a/tests/midhook.rs b/tests/midhook.rs index 01c3788..d449a89 100644 --- a/tests/midhook.rs +++ b/tests/midhook.rs @@ -8,8 +8,8 @@ //! restore, continue - is identical on both architectures and is covered by the //! "called" and "unhook restores" tests on every target. -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::Mutex; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use neohook::{HookContext, MidHook}; @@ -99,7 +99,10 @@ fn handler_runs_and_function_still_returns_correctly() { // The detour fires, then the original instructions resume unchanged. let result = noop_work(41); assert!(HANDLER_RAN.load(Ordering::SeqCst), "handler must have run"); - assert_eq!(result, 42, "original computation must complete after the detour"); + assert_eq!( + result, 42, + "original computation must complete after the detour" + ); hook.unhook().expect("unhook should succeed"); @@ -118,8 +121,7 @@ fn handler_observes_live_argument_register() { let _serial = install_guard(); OBSERVED_ARG.store(0, Ordering::SeqCst); - let hook = unsafe { MidHook::install(triple as *const u8, observe_handler) } - .expect("install"); + let hook = unsafe { MidHook::install(triple as *const u8, observe_handler) }.expect("install"); let _ = triple(7); assert_eq!( @@ -137,8 +139,8 @@ fn handler_can_rewrite_a_register_in_flight() { let _serial = install_guard(); assert_eq!(triple(5), 15); // baseline: 5 * 3 - let hook = unsafe { MidHook::install(triple as *const u8, add_to_arg_handler) } - .expect("install"); + let hook = + unsafe { MidHook::install(triple as *const u8, add_to_arg_handler) }.expect("install"); // Handler bumps RCX (the argument) by 10 before the body runs: (5 + 10) * 3. assert_eq!(triple(5), 45, "register edit must take effect"); @@ -195,8 +197,8 @@ fn dropping_the_guard_unhooks() { HANDLER_RAN.store(false, Ordering::SeqCst); { - let _hook = unsafe { MidHook::install(noop_work as *const u8, observe_handler) } - .expect("install"); + let _hook = + unsafe { MidHook::install(noop_work as *const u8, observe_handler) }.expect("install"); let _ = noop_work(1); assert!(HANDLER_RAN.load(Ordering::SeqCst)); } // _hook drops here -> original bytes restored diff --git a/tests/pattern_scan.rs b/tests/pattern_scan.rs index a0f646b..a5bc268 100644 --- a/tests/pattern_scan.rs +++ b/tests/pattern_scan.rs @@ -29,7 +29,8 @@ extern "system" fn add_detour(a: i32, b: i32) -> i32 { /// File name (without path) of the test executable, e.g. `pattern_scan-XXXX.exe`. fn main_module_name() -> String { let mut buf = [0u16; 1024]; - let len = unsafe { GetModuleFileNameW(std::ptr::null_mut(), buf.as_mut_ptr(), buf.len() as u32) }; + let len = + unsafe { GetModuleFileNameW(std::ptr::null_mut(), buf.as_mut_ptr(), buf.len() as u32) }; assert!(len > 0, "GetModuleFileNameW failed"); let full = String::from_utf16_lossy(&buf[..len as usize]); full.rsplit(['\\', '/']).next().unwrap_or(&full).to_string() @@ -68,7 +69,10 @@ fn scan_range_round_trips_a_local_function() { // Scan a window that comfortably contains the function. let found = unsafe { scan_range(target, 256, &pat) }.expect("signature should resolve"); - assert_eq!(found, target, "scan_range should land on the function start"); + assert_eq!( + found, target, + "scan_range should land on the function start" + ); } #[test] @@ -147,8 +151,8 @@ fn attach_pattern_reports_not_found() { #[test] fn scan_module_by_name_matches_get_module_handle_path() { // A signature taken from a real kernel32 export must resolve to that export. - let target = neohook::find_function("kernel32.dll", "GetProcAddress") - .expect("GetProcAddress resolves"); + let target = + neohook::find_function("kernel32.dll", "GetProcAddress").expect("GetProcAddress resolves"); let pat = signature_from(target, 12); let h = get_module_handle("kernel32.dll").expect("kernel32 handle"); diff --git a/tests/veh_hooks.rs b/tests/veh_hooks.rs index 44e3eff..ba2486b 100644 --- a/tests/veh_hooks.rs +++ b/tests/veh_hooks.rs @@ -33,15 +33,19 @@ fn detour_ptr() -> *const u8 { fn veh_hook_redirects_and_restores_on_calling_thread() { assert_eq!(call_target(), 1234, "precondition"); - let hook = unsafe { VehHook::install(target_ptr(), detour_ptr()) } - .expect("install should succeed"); + let hook = + unsafe { VehHook::install(target_ptr(), detour_ptr()) }.expect("install should succeed"); assert_eq!(hook.target(), target_ptr()); assert_eq!(hook.detour(), detour_ptr()); assert_eq!(call_target(), 9999, "calling thread should hit the detour"); hook.unhook().expect("unhook should succeed"); - assert_eq!(call_target(), 1234, "original must be restored after unhook"); + assert_eq!( + call_target(), + 1234, + "original must be restored after unhook" + ); } #[test] @@ -52,7 +56,11 @@ fn veh_hook_drop_restores() { assert_eq!(call_target(), 9999); // _hook drops here, clearing the breakpoint. } - assert_eq!(call_target(), 1234, "dropping the guard should restore the original"); + assert_eq!( + call_target(), + 1234, + "dropping the guard should restore the original" + ); } #[test] @@ -72,8 +80,8 @@ fn veh_hook_covers_threads_that_exist_at_install_time() { ready_rx.recv().unwrap(); - let hook = unsafe { VehHook::install(target_ptr(), detour_ptr()) } - .expect("install should succeed"); + let hook = + unsafe { VehHook::install(target_ptr(), detour_ptr()) }.expect("install should succeed"); go_tx.send(()).unwrap(); let worker_result = result_rx.recv().unwrap(); From 4e9efba98f4f0a3d72b3ebc7816764e43d045e53 Mon Sep 17 00:00:00 2001 From: AlienDwarf <61460778+AlienDwarf@users.noreply.github.com> Date: Sat, 20 Jun 2026 03:23:39 +0200 Subject: [PATCH 3/5] fix: satisfy clippy -D warnings on both CI targets CI runs `cargo clippy --all-targets --all-features -- -D warnings` for x86_64 and i686, which turns lints into hard errors: - delay.rs: collapse a nested `if` into a let-chain. - registry.rs: replace `assert_eq!(.., true/false)` with `assert!`. - tests/midhook.rs: the `triple` helper and `OBSERVED_ARG` static are only used by the x86_64-only register tests, so gate them (and the AtomicU64 import) behind `target_arch = "x86_64"` to avoid dead-code errors on i686. --- src/delay.rs | 8 ++++---- src/registry.rs | 15 ++++++++++++--- tests/midhook.rs | 9 ++++++++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/delay.rs b/src/delay.rs index d9d722a..73e14cd 100644 --- a/src/delay.rs +++ b/src/delay.rs @@ -183,10 +183,10 @@ impl DelayHook { } } // Once no delay hooks remain, retire the LdrLoadDll inline hook. - if mgr.pending.is_empty() { - if let Some(hook) = mgr.ldr_hook.take() { - let _ = hook.unhook(); - } + if mgr.pending.is_empty() + && let Some(hook) = mgr.ldr_hook.take() + { + let _ = hook.unhook(); } } } diff --git a/src/registry.rs b/src/registry.rs index c915e1d..2d51fca 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -195,15 +195,24 @@ mod tests { assert_eq!(target(), 77, "hook should be active once registered"); assert_eq!(is_enabled("reg-a"), Some(true)); - assert_eq!(disable("reg-a").unwrap(), true); + assert!( + disable("reg-a").unwrap(), + "disable should report it was enabled" + ); assert_eq!(target(), 7, "disabled hook should not intercept"); assert_eq!(is_enabled("reg-a"), Some(false)); - assert_eq!(enable("reg-a").unwrap(), true); + assert!( + enable("reg-a").unwrap(), + "enable should report it was disabled" + ); assert_eq!(target(), 77, "re-enabled hook should intercept again"); // Unknown names are reported, not errors. - assert_eq!(enable("nope").unwrap(), false); + assert!( + !enable("nope").unwrap(), + "enabling an unknown name reports false" + ); assert_eq!(is_enabled("nope"), None); let torn = unhook_all(); diff --git a/tests/midhook.rs b/tests/midhook.rs index d449a89..fa72c1d 100644 --- a/tests/midhook.rs +++ b/tests/midhook.rs @@ -9,7 +9,10 @@ //! "called" and "unhook restores" tests on every target. use std::sync::Mutex; -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, Ordering}; +// AtomicU64 backs the register-observation statics, which are x86_64-only. +#[cfg(target_arch = "x86_64")] +use std::sync::atomic::AtomicU64; use neohook::{HookContext, MidHook}; @@ -26,6 +29,9 @@ fn install_guard() -> std::sync::MutexGuard<'static, ()> { // A function whose entry we treat as an arbitrary code address to detour. // `black_box` keeps the compiler from constant-folding or inlining it away. +// Only the register-observing/rewriting tests use it, and those are x86_64-only +// (x86 passes integer arguments on the stack), so gate it to avoid dead code. +#[cfg(target_arch = "x86_64")] #[inline(never)] extern "system" fn triple(x: u64) -> u64 { std::hint::black_box(x).wrapping_mul(3) @@ -37,6 +43,7 @@ extern "system" fn noop_work(x: u64) -> u64 { } static HANDLER_RAN: AtomicBool = AtomicBool::new(false); +#[cfg(target_arch = "x86_64")] static OBSERVED_ARG: AtomicU64 = AtomicU64::new(0); unsafe extern "system" fn observe_handler(ctx: *mut HookContext) { From 019a06c968038d1324394fa0094fd8517b216428 Mon Sep 17 00:00:00 2001 From: AlienDwarf <61460778+AlienDwarf@users.noreply.github.com> Date: Sat, 20 Jun 2026 05:05:08 +0200 Subject: [PATCH 4/5] feat: allow a MidHook handler to redirect control flow Add HookContext::redirect_rip (redirect_eip on x86): a field that is zero on entry and, when a handler sets it to a code address, makes the context bridge jump there after restoring the (possibly modified) register/XMM state instead of continuing the original function - skipping the stolen instructions. The stub now reserves a zeroed redirect slot at the top of the frame, and after the handler returns selects redirect_rip (if non-zero) or the gateway via cmov and writes it into that slot. Because the slot is the topmost field, the register restore leaves rsp on it and a single indirect jmp transfers control. The jump is a jmp, not a ret, so it leaves the CET shadow stack intact; on x86 the slot is padded to 8 bytes so the frame matches size_of::(). Add MidHook::resume_address() (= target + stolen_len) so a handler can redirect past the patched region and otherwise continue normally. Runtime tests cover redirecting a hooked entry to a same-ABI replacement on both x86_64 and x86; layout/encoding tests pin the new slot offset and assert the stub stays ret-free. --- README.md | 19 ++-- include/neohook.h | 20 ++++ src/midhook.rs | 231 ++++++++++++++++++++++++++++++++++++---------- tests/midhook.rs | 65 +++++++++++++ 4 files changed, 282 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 3f3dd67..185e0e9 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ Function hooking is deceptively difficult to get right. Writing a `JMP` patch is | v0.8.0 | ✅ Done | Delay / on-load hooks (`DelayHook`) | | v0.8.0 | ✅ Done | Named hook registry (`registry`) | | v0.9.0 | ✅ Done | XMM / MXCSR context capture in `MidHook` | -| v0.9.0 | Planned | Control-flow redirect from a `MidHook` handler | +| v0.9.0 | ✅ Done | Control-flow redirect from a `MidHook` handler | | v0.9.0 | Planned | ARM64 inline hooking | -- @@ -405,11 +405,18 @@ fn main() { A handler may read any field of `HookContext` to observe a live register, or write one to change it before execution continues - including the floating-point / SIMD argument registers via `ctx.xmm[..]` (e.g. -`f64::from_bits(ctx.xmm[0].low)` for a scalar `double`). The detour always -*continues* the original function - it cannot skip it or redirect control flow. -General-purpose registers, flags, all XMM registers and `MXCSR` are captured; -the legacy x87 stack registers are not. `target` must sit on a real instruction -boundary. +`f64::from_bits(ctx.xmm[0].low)` for a scalar `double`). General-purpose +registers, flags, all XMM registers and `MXCSR` are captured; the legacy x87 +stack registers are not. `target` must sit on a real instruction boundary. + +By default the detour *continues* the original function. A handler can instead +**redirect control flow** by setting `ctx.redirect_rip` (`redirect_eip` on x86) +to a code address: the stub restores the (possibly modified) state and jumps +there, skipping the stolen instructions. Use it to replace a routine wholesale +(redirect a hooked entry to a same-ABI drop-in that returns to the caller) or to +skip the patched region via `hook.resume_address()` (`= target + stolen_len`). +The redirect is an indirect `jmp`, not a `ret`, so it leaves the CET shadow +stack intact. The C ABI exposes `detours_midhook_install(target, handler)` and `detours_midhook_unhook(hook)`; the C++ wrapper provides an RAII `neohook::MidHook`. diff --git a/include/neohook.h b/include/neohook.h index b9d8480..efb52c8 100644 --- a/include/neohook.h +++ b/include/neohook.h @@ -99,6 +99,10 @@ typedef struct Xmm { * `xmm` holds `XMM0`..`XMM15` in order, and `mxcsr` holds the SSE * control/status word. The x87 stack registers are not captured (see the * module-level limits). + * + * `redirect_rip` is **0 on entry**. Leave it 0 to continue the original + * function normally; set it to a code address to redirect control flow there + * instead (see the [module docs](self#control-flow-redirect)). */ typedef struct HookContext { /** @@ -132,6 +136,12 @@ typedef struct HookContext { * `XMM0` through `XMM15`, in register-number order. */ struct Xmm xmm[16]; + /** + * Control-flow redirect target. `0` (the default) continues the original + * function; any other value is jumped to after the registers are restored, + * skipping the stolen instructions. See the [module docs](self#control-flow-redirect). + */ + uint64_t redirect_rip; } HookContext; #endif @@ -144,6 +154,10 @@ typedef struct HookContext { * by the saved `MXCSR` and `XMM0`..`XMM7`. Writing `esp` has no effect * (`popad` discards its saved stack pointer slot). The x87 stack registers are * not captured (see the module-level limits). + * + * `redirect_eip` is **0 on entry**. Leave it 0 to continue the original + * function normally; set it to a code address to redirect control flow there + * instead (see the [module docs](self#control-flow-redirect)). */ typedef struct HookContext { /** @@ -166,6 +180,12 @@ typedef struct HookContext { * `XMM0` through `XMM7`, in register-number order. */ struct Xmm xmm[8]; + /** + * Control-flow redirect target. `0` (the default) continues the original + * function; any other value is jumped to after the registers are restored, + * skipping the stolen instructions. See the [module docs](self#control-flow-redirect). + */ + uint32_t redirect_eip; } HookContext; #endif diff --git a/src/midhook.rs b/src/midhook.rs index cab84b2..3a38f85 100644 --- a/src/midhook.rs +++ b/src/midhook.rs @@ -25,8 +25,9 @@ //! handler(&mut HookContext) ; your code reads / modifies the snapshot //! pop all GPRs + flags ; apply any integer edits back //! restore XMM regs + MXCSR ; apply any SSE / FP edits back -//! ; run the original instructions -//! JMP target + stolen_len ; resume the function +//! JMP (redirect ? handler-set address : gateway) +//! ; gateway runs the relocated stolen bytes +//! ; and resumes at target + stolen_len //! ``` //! //! Your handler receives a pointer to a [`HookContext`] mirroring the saved @@ -37,6 +38,24 @@ //! SIMD arguments, results, loop counters, or flags in flight, **without** the //! function ever returning to a caller. //! +//! # Control-flow redirect +//! +//! By default the detour *continues* the original function: after the handler +//! returns, the stub runs the relocated stolen bytes and resumes at +//! `target + stolen_len`. A handler can instead **redirect control flow** by +//! setting [`HookContext::redirect_rip`] (`redirect_eip` on x86) to a code +//! address. When that field is non-zero, the stub restores the (possibly +//! modified) register/XMM state and jumps straight there, **skipping the stolen +//! instructions entirely**. The handler-chosen target therefore executes with +//! exactly the register and stack state the context describes. +//! +//! Typical uses: replace a routine wholesale (hook its entry, redirect to a +//! drop-in with the same ABI, which returns to the original caller), or skip a +//! patched region and continue at [`MidHook::resume_address`] +//! (`= target + stolen_len`, where the default path would have resumed). The +//! redirect uses an indirect `jmp`, not a `ret`, so it does not disturb the +//! CPU return-address predictor or a CET shadow stack. +//! //! The patch itself reuses the full inline-hook engine //! ([`crate::transaction`]): threads are suspended, any instruction pointer or //! return address inside the overwritten range is redirected, the stolen bytes @@ -45,9 +64,11 @@ //! //! # Safety and limits //! -//! - The detour always *continues* the original function; there is no facility -//! to skip it or redirect control flow from the handler (modifying the saved -//! instruction pointer is intentionally not supported). +//! - Control flow can be redirected via [`HookContext::redirect_rip`] (see +//! above), but the redirect target is reached with the *current* stack - the +//! stub neither unwinds nor adjusts `rsp`. You are responsible for choosing a +//! target that is valid for that stack state (e.g. a same-ABI replacement of +//! the function whose entry you hooked, or [`MidHook::resume_address`]). //! - General-purpose registers, the flags register, all XMM registers and the //! `MXCSR` word are captured and restored. The legacy **x87** stack registers //! (`st0`-`st7`) and MMX state are **not** snapshotted; if your handler runs @@ -64,8 +85,8 @@ use crate::alloc::{Trampoline, TrampolineAlloc}; use crate::transaction::{Hook, TransactionCore}; /// Capacity reserved for the generated context-bridge stub. With the XMM / -/// MXCSR save-restore block the real stub is ~380 bytes on x86_64 and ~150 on -/// x86; this leaves generous headroom. +/// MXCSR save-restore block and the redirect dispatch the real stub is ~420 +/// bytes on x86_64 and ~180 on x86; this leaves generous headroom. const STUB_CAPACITY: usize = 512; /// A 128-bit XMM register, captured for a [`MidHook`] handler. @@ -97,6 +118,10 @@ pub struct Xmm { /// `xmm` holds `XMM0`..`XMM15` in order, and `mxcsr` holds the SSE /// control/status word. The x87 stack registers are not captured (see the /// module-level limits). +/// +/// `redirect_rip` is **0 on entry**. Leave it 0 to continue the original +/// function normally; set it to a code address to redirect control flow there +/// instead (see the [module docs](self#control-flow-redirect)). #[cfg(target_arch = "x86_64")] #[repr(C)] #[derive(Clone, Copy, Debug)] @@ -124,6 +149,10 @@ pub struct HookContext { pub _reserved: u32, /// `XMM0` through `XMM15`, in register-number order. pub xmm: [Xmm; 16], + /// Control-flow redirect target. `0` (the default) continues the original + /// function; any other value is jumped to after the registers are restored, + /// skipping the stolen instructions. See the [module docs](self#control-flow-redirect). + pub redirect_rip: u64, } /// Snapshot of the general-purpose registers, flags and SSE / floating-point @@ -133,6 +162,10 @@ pub struct HookContext { /// by the saved `MXCSR` and `XMM0`..`XMM7`. Writing `esp` has no effect /// (`popad` discards its saved stack pointer slot). The x87 stack registers are /// not captured (see the module-level limits). +/// +/// `redirect_eip` is **0 on entry**. Leave it 0 to continue the original +/// function normally; set it to a code address to redirect control flow there +/// instead (see the [module docs](self#control-flow-redirect)). #[cfg(target_arch = "x86")] #[repr(C)] #[derive(Clone, Copy, Debug)] @@ -151,6 +184,10 @@ pub struct HookContext { pub mxcsr: u32, /// `XMM0` through `XMM7`, in register-number order. pub xmm: [Xmm; 8], + /// Control-flow redirect target. `0` (the default) continues the original + /// function; any other value is jumped to after the registers are restored, + /// skipping the stolen instructions. See the [module docs](self#control-flow-redirect). + pub redirect_eip: u32, } /// A handler invoked at a mid-function hook site with a pointer to the captured @@ -176,6 +213,8 @@ pub struct MidHook { #[allow(dead_code)] stub: Trampoline, target: *mut u8, + /// `target + stolen_len`: where the default (non-redirecting) path resumes. + resume: *mut u8, } impl MidHook { @@ -250,10 +289,20 @@ impl MidHook { .next() .ok_or(DetourError::InvalidParameter)?; + // The number of bytes stolen at the target; the default path resumes at + // `target + stolen_len`. A mid-function patch always produces an inline + // hook, but fall back to 0 defensively for any other variant. + let stolen_len = match &hook { + Hook::Inline(h) => h.stolen_len, + _ => 0, + }; + let resume = unsafe { target.add(stolen_len) }; + Ok(MidHook { hook: Some(hook), stub, target, + resume, }) } @@ -262,6 +311,19 @@ impl MidHook { self.target } + /// Returns the address where the original function resumes when a handler + /// does **not** redirect - i.e. `target + stolen_len`, just past the bytes + /// the patch overwrote. + /// + /// A handler can set [`HookContext::redirect_rip`] (`redirect_eip` on x86) + /// to this value to skip the patched region but otherwise let the function + /// continue normally. Capture it at install time (e.g. into a `static`) so + /// the handler can read it. Note this skips the *relocated stolen bytes*, so + /// only resume here when those instructions are safe to omit. + pub fn resume_address(&self) -> *const u8 { + self.resume + } + /// Removes the detour, restoring the original bytes at the target. The /// context-bridge stub is released when the returned value is dropped. pub fn unhook(mut self) -> Result<(), DetourError> { @@ -278,16 +340,22 @@ impl MidHook { /// Emits the context-bridge stub: snapshot the SSE state (XMM + MXCSR) and all /// GPRs + flags, call `handler` with a pointer to that block, restore the -/// (possibly modified) state, then jump to `gateway` (which runs the relocated -/// stolen bytes and resumes the function). `stub_addr` is the address the bytes -/// will live at. +/// (possibly modified) state, then jump either to `gateway` (which runs the +/// relocated stolen bytes and resumes the function) or, if the handler set +/// `redirect_rip`, straight to that address. `stub_addr` is the address the +/// bytes will live at. /// /// The combined block, from the lowest address (where the handler pointer aims) -/// upward, is `rflags, rax..r15, mxcsr, _pad, xmm0..xmm15` - exactly the -/// [`HookContext`] layout. The save order is therefore "highest field first": -/// XMM (with `xmm15` pushed first so `xmm0` ends up lowest), then MXCSR, then -/// the GPRs, then flags last. `movups` is used so the stack need not be -/// 16-byte aligned for the spill. +/// upward, is `rflags, rax..r15, mxcsr, _pad, xmm0..xmm15, redirect_rip` - +/// exactly the [`HookContext`] layout. The save order is therefore "highest +/// field first": the (zeroed) `redirect_rip` slot, then XMM (with `xmm15` pushed +/// first so `xmm0` ends up lowest), then MXCSR, then the GPRs, then flags last. +/// `movups` is used so the stack need not be 16-byte aligned for the spill. +/// +/// After the handler returns, the stub overwrites the `redirect_rip` slot with +/// the chosen target (`redirect_rip` if non-zero, else `gateway`); that slot is +/// the very top of the frame, so once the registers are restored `rsp` lands on +/// it and a single indirect `jmp` transfers control. #[cfg(target_arch = "x86_64")] fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec { let _ = stub_addr; // x64 stub is position-independent. @@ -310,6 +378,12 @@ fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec Vec Vec --- clobbers no register. - c.extend_from_slice(&[0xFF, 0x25, 0x00, 0x00, 0x00, 0x00]); - c.extend_from_slice(&(gateway as u64).to_le_bytes()); + // After the XMM reloads rsp points at the redirect slot (= the chosen + // target). Drop it back to the entry rsp and jump through it. Using an + // indirect `jmp` (not `ret`) keeps the CET shadow stack intact; the operand + // sits 8 bytes below rsp only for this single instruction. + c.extend_from_slice(&[0x48, 0x83, 0xC4, 0x08]); // add rsp, 8 (rsp = entry rsp) + c.extend_from_slice(&[0xFF, 0x64, 0x24, 0xF8]); // jmp qword [rsp - 8] c } @@ -392,6 +481,14 @@ fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec()` (the struct is 8-aligned + // because of `Xmm`), leaving no slack a full-struct read could run past. + c.extend_from_slice(&[0x83, 0xEC, 0x08]); // sub esp, 8 + c.extend_from_slice(&[0xC7, 0x04, 0x24, 0x00, 0x00, 0x00, 0x00]); // mov dword [esp], 0 (redirect_eip) + // --- save: xmm7..xmm0 (xmm0 ends lowest), MXCSR, then pushad + pushfd --- for n in (0..8u8).rev() { save_xmm(&mut c, n); @@ -408,6 +505,19 @@ fn build_stub(stub_addr: *mut u8, handler: *const u8, gateway: *mut u8) -> Vec Vec(), 136 + 16 * 16); + // redirect_rip must be the topmost field at offset 392 - the stub's + // hard-coded REDIRECT_OFF and the slot it jumps through depend on it. + assert_eq!(offset_of!(HookContext, redirect_rip), 392); + assert_eq!(size_of::(), 392 + 8); } #[cfg(target_arch = "x86")] @@ -522,28 +645,42 @@ mod tests { assert_eq!(offset_of!(HookContext, eax), 32); assert_eq!(offset_of!(HookContext, mxcsr), 36); assert_eq!(offset_of!(HookContext, xmm), 40); - assert_eq!(size_of::(), 40 + 8 * 16); + // redirect_eip must be the topmost field at offset 168 (the stub's + // REDIRECT_OFF and ret-free jump slot depend on it). The struct is + // 8-aligned (Xmm), so it is padded to 176 bytes - the stub reserves the + // matching 8-byte slot. + assert_eq!(offset_of!(HookContext, redirect_eip), 168); + assert_eq!(size_of::(), 176); } #[cfg(target_arch = "x86")] #[test] - fn x86_stub_encodes_relative_jump_to_gateway() { - let stub_addr = 0x0040_0000i64; - let gateway = 0x0050_0000i64; + fn x86_stub_redirects_through_slot_without_ret() { + let gateway = 0x0050_0000u32; let bytes = build_stub( - stub_addr as *mut u8, + 0x0040_0000 as *mut u8, 0xCAFE as *const u8, gateway as *mut u8, ); - // The XMM spill prologue now runs before pushad; the first thing the - // stub does is reserve a 16-byte slot (sub esp, 16) for xmm7. - assert_eq!(&bytes[0..3], &[0x83, 0xEC, 0x10], "first op is sub esp, 16"); + // First op reserves the 8-byte redirect slot (sub esp, 8). + assert_eq!(&bytes[0..3], &[0x83, 0xEC, 0x08], "first op is sub esp, 8"); assert!(bytes.contains(&0x60), "pushad present"); - assert_eq!(*bytes.last_chunk::<5>().unwrap().first().unwrap(), 0xE9); - let rel = i32::from_le_bytes(bytes[bytes.len() - 4..].try_into().unwrap()) as i64; - let next_ip = stub_addr + bytes.len() as i64; - assert_eq!(next_ip + rel, gateway, "rel32 must resolve to the gateway"); + // The gateway is embedded as imm32 after `mov ecx, imm32` (B9). + let movimm = bytes + .windows(1) + .position(|w| w == [0xB9]) + .expect("mov ecx, imm32 present"); + let gw = u32::from_le_bytes(bytes[movimm + 1..movimm + 5].try_into().unwrap()); + assert_eq!(gw, gateway); + + // Tail is `add esp, 8` then indirect `jmp [esp - 8]` (FF 64 24 F8), no ret. + assert_eq!( + &bytes[bytes.len() - 4..], + &[0xFF, 0x64, 0x24, 0xF8], + "stub tail must be jmp dword [esp-8]" + ); + assert!(!bytes.contains(&0xC3), "stub must not contain a ret (0xC3)"); } } diff --git a/tests/midhook.rs b/tests/midhook.rs index fa72c1d..bf43f3c 100644 --- a/tests/midhook.rs +++ b/tests/midhook.rs @@ -192,6 +192,71 @@ fn handler_can_rewrite_an_xmm_register_in_flight() { assert_eq!(add_half(1.0), 1.5, "behaviour restored after unhook"); } +// --- Control-flow redirect (works on both architectures) --------------------- + +#[inline(never)] +extern "system" fn redir_original(x: u64) -> u64 { + std::hint::black_box(x).wrapping_mul(3) +} + +// A drop-in replacement with the same ABI. Redirecting `redir_original`'s entry +// here makes the call behave as if `redir_replacement` had been called: it runs +// with the same arguments/stack and returns to the original caller. +#[inline(never)] +extern "system" fn redir_replacement(x: u64) -> u64 { + std::hint::black_box(x).wrapping_add(1000) +} + +unsafe extern "system" fn redirect_handler(ctx: *mut HookContext) { + let ctx = unsafe { &mut *ctx }; + #[cfg(target_arch = "x86_64")] + { + ctx.redirect_rip = redir_replacement as usize as u64; + } + #[cfg(target_arch = "x86")] + { + ctx.redirect_eip = redir_replacement as usize as u32; + } +} + +#[test] +fn handler_can_redirect_control_flow() { + let _serial = install_guard(); + assert_eq!(redir_original(5), 15); // baseline: 5 * 3 + + let hook = unsafe { MidHook::install(redir_original as *const u8, redirect_handler) } + .expect("install"); + + // The handler redirects to redir_replacement, skipping the original body: + // 5 + 1000. + assert_eq!( + redir_original(5), + 1005, + "control flow must be redirected to the replacement" + ); + + hook.unhook().expect("unhook"); + assert_eq!(redir_original(5), 15, "original restored after unhook"); +} + +#[test] +fn resume_address_is_past_the_patched_bytes() { + let _serial = install_guard(); + let hook = + unsafe { MidHook::install(redir_original as *const u8, observe_handler) }.expect("install"); + + let target = hook.target() as usize; + let resume = hook.resume_address() as usize; + // resume == target + stolen_len; the patch steals at least a 5-byte jmp and + // never an unreasonable amount. + assert!( + resume > target && resume - target >= 5 && resume - target <= 24, + "resume_address ({resume:#x}) must sit just past the patch at target ({target:#x})" + ); + + hook.unhook().expect("unhook"); +} + #[test] fn install_rejects_null_target() { let err = unsafe { MidHook::install(std::ptr::null(), observe_handler) }; From 333ba9695d611c3b20b864b4052dde44494949a0 Mon Sep 17 00:00:00 2001 From: AlienDwarf <61460778+AlienDwarf@users.noreply.github.com> Date: Sat, 20 Jun 2026 17:05:11 +0200 Subject: [PATCH 5/5] format update --- src/pe.rs | 89 +++++ tests/error_messages.rs | 163 +++++++++ tests/ffi_introspection.rs | 682 +++++++++++++++++++++++++++++++++++++ 3 files changed, 934 insertions(+) create mode 100644 tests/error_messages.rs create mode 100644 tests/ffi_introspection.rs diff --git a/src/pe.rs b/src/pe.rs index 38fca37..2e08ec9 100644 --- a/src/pe.rs +++ b/src/pe.rs @@ -193,3 +193,92 @@ pub fn read_c_string_from_rva(image: &ModuleImage, rva: usize) -> Option None } + +#[cfg(test)] +mod tests { + use super::*; + + /// A DOS header immediately followed by the NT headers, laid out the same + /// way a real loaded module is. Driving `parse_module_image` against a + /// pointer to one of these lets each validation branch be hit deliberately + /// by corrupting a single field of an otherwise-valid image. + #[repr(C)] + struct FakeImage { + dos: IMAGE_DOS_HEADER, + nt: IMAGE_NT_HEADERS, + } + + /// Builds a synthetic image whose headers pass every check, so each test + /// can corrupt exactly one field and assert the resulting `PeError`. + fn valid_image() -> Box { + // SAFETY: `IMAGE_DOS_HEADER` / `IMAGE_NT_HEADERS` are plain C POD + // structs, so an all-zero instance is a sound starting point that we + // then populate with the few fields the parser inspects. + let mut img: Box = Box::new(unsafe { std::mem::zeroed() }); + img.dos.e_magic = IMAGE_DOS_SIGNATURE; + img.dos.e_lfanew = std::mem::offset_of!(FakeImage, nt) as i32; + img.nt.Signature = IMAGE_NT_SIGNATURE; + img.nt.OptionalHeader.Magic = IMAGE_OPTIONAL_MAGIC; + img.nt.OptionalHeader.SizeOfImage = std::mem::size_of::() as u32; + img + } + + fn parse(img: &FakeImage) -> Result { + parse_module_image(img as *const FakeImage as HMODULE) + } + + #[test] + fn valid_headers_parse() { + let img = valid_image(); + let parsed = parse(&img).expect("a well-formed image must parse"); + assert_eq!(parsed.base_address, &*img as *const FakeImage as usize); + assert_eq!(parsed.size, std::mem::size_of::()); + } + + #[test] + fn null_module_is_rejected() { + assert!(matches!( + parse_module_image(std::ptr::null_mut()), + Err(PeError::NullModule) + )); + } + + #[test] + fn missing_dos_signature_is_rejected() { + let mut img = valid_image(); + img.dos.e_magic = 0; // not "MZ" + assert!(matches!(parse(&img), Err(PeError::InvalidDosHeader))); + } + + #[test] + fn non_positive_e_lfanew_is_rejected() { + // Zero and negative offsets to the NT headers are both invalid. + for bad in [0, -1] { + let mut img = valid_image(); + img.dos.e_lfanew = bad; + assert!(matches!(parse(&img), Err(PeError::InvalidNtHeader))); + } + } + + #[test] + fn missing_nt_signature_is_rejected() { + let mut img = valid_image(); + img.nt.Signature = 0; // not "PE\0\0" + assert!(matches!(parse(&img), Err(PeError::InvalidNtSignature))); + } + + #[test] + fn wrong_optional_header_magic_is_rejected() { + let mut img = valid_image(); + // Flip to the magic of the *other* architecture so it never matches. + img.nt.OptionalHeader.Magic = !IMAGE_OPTIONAL_MAGIC; + assert!(matches!(parse(&img), Err(PeError::InvalidOptionalHeader))); + } + + #[test] + fn zero_size_of_image_is_rejected() { + let mut img = valid_image(); + img.nt.OptionalHeader.SizeOfImage = 0; + assert!(matches!(parse(&img), Err(PeError::InvalidOptionalHeader))); + } +} diff --git a/tests/error_messages.rs b/tests/error_messages.rs new file mode 100644 index 0000000..767c5ac --- /dev/null +++ b/tests/error_messages.rs @@ -0,0 +1,163 @@ +#![cfg(windows)] + +//! Exhaustive coverage of the `Display` / `From` / `Error::source` surfaces of +//! every public error type. These arms are otherwise only reached on rare OS +//! failures, so they are pinned here directly by constructing each variant. + +use neohook::{ + DelayHookError, DetourError, EatHookError, HookKind, IatHookError, Int3HookError, PatternError, + VTableHookError, VehHookError, +}; + +fn io_err() -> std::io::Error { + std::io::Error::from_raw_os_error(5) // ERROR_ACCESS_DENIED +} + +#[test] +fn hook_kind_display_covers_every_variant() { + assert_eq!(HookKind::Inline.to_string(), "inline"); + assert_eq!(HookKind::Iat.to_string(), "IAT"); + assert_eq!(HookKind::Eat.to_string(), "EAT"); + assert_eq!(HookKind::Vtable.to_string(), "VTable"); + assert_eq!(HookKind::VtableInstance.to_string(), "per-instance VTable"); + assert_eq!(HookKind::Detach.to_string(), "detach"); +} + +#[test] +fn detour_error_display_covers_every_variant() { + let cases: Vec = vec![ + DetourError::NotStarted, + DetourError::AllocationFailed, + DetourError::RelocationFailed, + DetourError::InvalidParameter, + DetourError::Pattern(PatternError::Empty), + DetourError::PatternNotFound, + DetourError::Iat(IatHookError::TargetNotFound), + DetourError::Eat(EatHookError::TargetIsForwarder), + DetourError::Vtable(VTableHookError::InvalidParameter), + DetourError::CommitFailed { + index: 0, + kind: HookKind::Inline, + source: Box::new(DetourError::AllocationFailed), + }, + ]; + for err in &cases { + assert!( + !err.to_string().is_empty(), + "every arm must format: {err:?}" + ); + } + + // Error::source is Some only for CommitFailed. + use std::error::Error; + assert!(DetourError::NotStarted.source().is_none()); + let nested = DetourError::CommitFailed { + index: 1, + kind: HookKind::Eat, + source: Box::new(DetourError::InvalidParameter), + }; + assert!(nested.source().is_some()); +} + +#[test] +fn detour_error_from_conversions() { + let from_iat: DetourError = IatHookError::InvalidParameter.into(); + assert!(matches!(from_iat, DetourError::Iat(_))); + + let from_pat: DetourError = PatternError::Empty.into(); + assert!(matches!(from_pat, DetourError::Pattern(_))); + + let from_eat: DetourError = EatHookError::TargetNotFound.into(); + assert!(matches!(from_eat, DetourError::Eat(_))); + + let from_vtable: DetourError = VTableHookError::AllocationFailed.into(); + assert!(matches!(from_vtable, DetourError::Vtable(_))); +} + +#[test] +fn iat_hook_error_display() { + for err in [ + IatHookError::InvalidParameter, + IatHookError::InvalidPeImage, + IatHookError::ImportTableUnavailable, + IatHookError::NameResolutionUnavailable, + IatHookError::TargetNotFound, + IatHookError::ProtectFailed(io_err()), + ] { + assert!(!err.to_string().is_empty()); + } +} + +#[test] +fn eat_hook_error_display() { + for err in [ + EatHookError::InvalidParameter, + EatHookError::InvalidPeImage, + EatHookError::ExportTableUnavailable, + EatHookError::TargetNotFound, + EatHookError::TargetIsForwarder, + EatHookError::DetourUnreachable, + EatHookError::AllocationFailed, + EatHookError::ProtectFailed(io_err()), + ] { + assert!(!err.to_string().is_empty()); + } +} + +#[test] +fn veh_hook_error_display() { + for err in [ + VehHookError::InvalidParameter, + VehHookError::NoFreeSlot, + VehHookError::AlreadyHooked, + VehHookError::HandlerRegistrationFailed, + ] { + assert!(!err.to_string().is_empty()); + } +} + +#[test] +fn int3_hook_error_display() { + for err in [ + Int3HookError::InvalidParameter, + Int3HookError::NoFreeSlot, + Int3HookError::AlreadyHooked, + Int3HookError::HandlerRegistrationFailed, + Int3HookError::PatchFailed, + ] { + assert!(!err.to_string().is_empty()); + } +} + +#[test] +fn vtable_hook_error_display() { + for err in [ + VTableHookError::InvalidParameter, + VTableHookError::AllocationFailed, + VTableHookError::ProtectFailed(io_err()), + ] { + assert!(!err.to_string().is_empty()); + } +} + +#[test] +fn delay_hook_error_display() { + for err in [ + DelayHookError::InvalidParameter, + DelayHookError::LdrHookFailed, + ] { + assert!(!err.to_string().is_empty()); + } +} + +#[test] +fn pattern_error_display_covers_every_variant() { + for err in [ + PatternError::Empty, + PatternError::InvalidToken("ZZ".to_string()), + PatternError::InvalidMaskChar('q'), + PatternError::MaskLengthMismatch { bytes: 3, mask: 2 }, + ] { + assert!(!err.to_string().is_empty()); + } +} diff --git a/tests/ffi_introspection.rs b/tests/ffi_introspection.rs new file mode 100644 index 0000000..a8932f5 --- /dev/null +++ b/tests/ffi_introspection.rs @@ -0,0 +1,682 @@ +#![cfg(windows)] + +//! FFI coverage for the C entry points that wrap module / PE introspection, +//! signature scanning, relative-reference resolving, and the standalone hook +//! engines (VEH, INT3, mid-function, delay). These thin `extern "C"` shims are +//! exercised here the same way a C consumer would call them: through raw +//! pointers and NULL-terminated strings, including the null / invalid-UTF-8 +//! rejection paths. + +use neohook::api::*; +use neohook::{HookContext, MidHookHandler}; +use std::ffi::{CStr, CString}; +use std::ptr; +use std::sync::atomic::{AtomicBool, Ordering}; +use windows_sys::Win32::Foundation::HMODULE; +use windows_sys::Win32::System::LibraryLoader::GetModuleHandleW; + +// --- shared targets ---------------------------------------------------------- + +#[inline(never)] +extern "system" fn scan_me(a: i32, b: i32) -> i32 { + std::hint::black_box(a) + std::hint::black_box(b) + std::hint::black_box(7) +} + +#[inline(never)] +extern "system" fn scan_detour(_a: i32, _b: i32) -> i32 { + 4242 +} + +#[inline(never)] +extern "system" fn veh_target() -> u32 { + std::hint::black_box(1234) +} + +extern "system" fn veh_detour() -> u32 { + 9999 +} + +#[inline(never)] +extern "system" fn int3_target() -> u32 { + std::hint::black_box(7) +} + +extern "system" fn int3_detour() -> u32 { + 77 +} + +#[inline(never)] +extern "system" fn mid_target(x: u64) -> u64 { + std::hint::black_box(x).wrapping_add(1) +} + +static MID_RAN: AtomicBool = AtomicBool::new(false); + +unsafe extern "system" fn mid_handler(_ctx: *mut HookContext) { + MID_RAN.store(true, Ordering::SeqCst); +} + +/// A NUL-terminated C string whose body is not valid UTF-8, used to drive the +/// `CStr::to_str()` rejection branches of the FFI shims. +fn invalid_utf8_cstr() -> &'static CStr { + c"\xff\xfe" +} + +fn kernel32() -> HMODULE { + let h = unsafe { GetModuleHandleW(windows_sys::core::w!("kernel32.dll")) }; + assert!(!h.is_null(), "kernel32 must be loaded"); + h +} + +/// Builds an all-fixed IDA signature from the first `len` bytes at `addr`. +fn signature_from(addr: *const u8, len: usize) -> CString { + let bytes = unsafe { std::slice::from_raw_parts(addr, len) }; + let ida = bytes + .iter() + .map(|b| format!("{b:02X}")) + .collect::>() + .join(" "); + CString::new(ida).unwrap() +} + +// --- detours_code_from_pointer ---------------------------------------------- + +#[test] +fn ffi_code_from_pointer_handles_real_and_null() { + unsafe { + let resolved = detours_code_from_pointer(scan_me as *const u8); + assert!(!resolved.is_null(), "a real code pointer must resolve"); + assert!(detours_code_from_pointer(ptr::null()).is_null()); + } +} + +// --- module enumeration ------------------------------------------------------ + +#[test] +fn ffi_enumerate_modules_exposes_fields_and_frees() { + unsafe { + let handle = detours_enumerate_modules(); + assert!(!handle.is_null()); + + let len = detours_modules_len(handle); + assert!(len > 0, "the process has at least one module"); + + // Every entry must surface a base, a non-empty name, and a plausible size. + let mut saw_named = false; + for i in 0..len { + let base = detours_modules_base(handle, i); + assert!(!base.is_null()); + let _size = detours_modules_size(handle, i); + let name = detours_modules_name(handle, i); + assert!(!name.is_null()); + if !CStr::from_ptr(name).to_bytes().is_empty() { + saw_named = true; + } + } + assert!(saw_named); + + // Out-of-bounds index is a defined null / zero, not a panic. + assert!(detours_modules_base(handle, len + 100).is_null()); + assert_eq!(detours_modules_size(handle, len + 100), 0); + assert!(detours_modules_name(handle, len + 100).is_null()); + + detours_modules_free(handle); + + // Null-handle guards. + assert_eq!(detours_modules_len(ptr::null_mut()), 0); + assert!(detours_modules_base(ptr::null_mut(), 0).is_null()); + assert_eq!(detours_modules_size(ptr::null_mut(), 0), 0); + assert!(detours_modules_name(ptr::null_mut(), 0).is_null()); + detours_modules_free(ptr::null_mut()); // no-op, must not crash + } +} + +// --- entry point ------------------------------------------------------------- + +#[test] +fn ffi_get_entry_point_for_null_and_kernel32() { + unsafe { + // Null module => main executable entry point. + assert!(!detours_get_entry_point(ptr::null_mut()).is_null()); + // A real module also has an entry point. + assert!(!detours_get_entry_point(kernel32() as *mut _).is_null()); + } +} + +// --- exports ----------------------------------------------------------------- + +#[test] +fn ffi_enumerate_exports_exposes_fields_and_frees() { + unsafe { + let handle = detours_enumerate_exports(kernel32() as *mut _); + assert!(!handle.is_null()); + + let len = detours_exports_len(handle); + assert!(len > 0, "kernel32 exports a lot"); + + let mut saw_name = false; + let mut saw_address = false; + for i in 0..len { + let _ord = detours_exports_ordinal(handle, i); + if !detours_exports_name(handle, i).is_null() { + saw_name = true; + } + if !detours_exports_address(handle, i).is_null() { + saw_address = true; + } + // forwarder is allowed to be null for most entries; just touch it. + let _ = detours_exports_forwarder(handle, i); + } + assert!(saw_name && saw_address); + + // Out-of-bounds. + assert_eq!(detours_exports_ordinal(handle, len + 100), 0); + assert!(detours_exports_name(handle, len + 100).is_null()); + assert!(detours_exports_address(handle, len + 100).is_null()); + assert!(detours_exports_forwarder(handle, len + 100).is_null()); + + detours_exports_free(handle); + + // Null guards. + assert_eq!(detours_exports_len(ptr::null_mut()), 0); + assert_eq!(detours_exports_ordinal(ptr::null_mut(), 0), 0); + assert!(detours_exports_name(ptr::null_mut(), 0).is_null()); + assert!(detours_exports_address(ptr::null_mut(), 0).is_null()); + assert!(detours_exports_forwarder(ptr::null_mut(), 0).is_null()); + detours_exports_free(ptr::null_mut()); + } +} + +#[test] +fn ffi_enumerate_exports_rejects_invalid_module() { + let mut junk = [0u8; 64]; + unsafe { + let handle = detours_enumerate_exports(junk.as_mut_ptr() as *mut _); + assert!(handle.is_null(), "garbage PE headers must be rejected"); + } +} + +// --- imports ----------------------------------------------------------------- + +#[test] +fn ffi_enumerate_imports_exposes_fields_and_frees() { + let h_exe: HMODULE = unsafe { GetModuleHandleW(ptr::null()) }; + assert!(!h_exe.is_null()); + unsafe { + let handle = detours_enumerate_imports(h_exe as *mut _); + assert!(!handle.is_null()); + + let len = detours_imports_len(handle); + assert!(len > 0, "the test exe imports from at least one DLL"); + + let mut saw_dll = false; + for i in 0..len { + let dll = detours_imports_dll(handle, i); + assert!(!dll.is_null()); + if !CStr::from_ptr(dll).to_bytes().is_empty() { + saw_dll = true; + } + // A given import is either by-name or by-ordinal. + let ord = detours_imports_ordinal(handle, i); + let name = detours_imports_name(handle, i); + assert!( + ord != u32::MAX || !name.is_null(), + "an import must have a name or an ordinal" + ); + let _ = detours_imports_address(handle, i); + } + assert!(saw_dll); + + // Out-of-bounds: ordinal sentinel is u32::MAX. + assert!(detours_imports_dll(handle, len + 100).is_null()); + assert!(detours_imports_name(handle, len + 100).is_null()); + assert_eq!(detours_imports_ordinal(handle, len + 100), u32::MAX); + assert!(detours_imports_address(handle, len + 100).is_null()); + + detours_imports_free(handle); + + // Null guards. + assert_eq!(detours_imports_len(ptr::null_mut()), 0); + assert!(detours_imports_dll(ptr::null_mut(), 0).is_null()); + assert!(detours_imports_name(ptr::null_mut(), 0).is_null()); + assert_eq!(detours_imports_ordinal(ptr::null_mut(), 0), u32::MAX); + assert!(detours_imports_address(ptr::null_mut(), 0).is_null()); + detours_imports_free(ptr::null_mut()); + } +} + +#[test] +fn ffi_enumerate_imports_rejects_invalid_module() { + let mut junk = [0u8; 64]; + unsafe { + assert!(detours_enumerate_imports(junk.as_mut_ptr() as *mut _).is_null()); + } +} + +// --- find_function / find_function_by_ordinal -------------------------------- + +#[test] +fn ffi_find_function_resolves_and_guards() { + let module = CString::new("kernel32.dll").unwrap(); + let func = CString::new("GetProcAddress").unwrap(); + unsafe { + assert!(!detours_find_function(module.as_ptr(), func.as_ptr()).is_null()); + + // Null guards. + assert!(detours_find_function(ptr::null(), func.as_ptr()).is_null()); + assert!(detours_find_function(module.as_ptr(), ptr::null()).is_null()); + + // Invalid UTF-8 in either argument. + assert!(detours_find_function(invalid_utf8_cstr().as_ptr(), func.as_ptr()).is_null()); + assert!(detours_find_function(module.as_ptr(), invalid_utf8_cstr().as_ptr()).is_null()); + + // Missing export resolves to null. + let missing = CString::new("NotAnExport_zzz").unwrap(); + assert!(detours_find_function(module.as_ptr(), missing.as_ptr()).is_null()); + } +} + +#[test] +fn ffi_find_function_by_ordinal_resolves_and_guards() { + let module = CString::new("kernel32.dll").unwrap(); + unsafe { + // Resolve a real ordinal by walking the export table, then look it up. + let handle = detours_enumerate_exports(kernel32() as *mut _); + assert!(!handle.is_null()); + let len = detours_exports_len(handle); + let mut resolved_any = false; + for i in 0..len { + if detours_exports_name(handle, i).is_null() { + continue; // by-ordinal-only entries are fine too, but skip for stability + } + let ord = detours_exports_ordinal(handle, i); + if ord == 0 || ord > u16::MAX as u32 { + continue; + } + if !detours_find_function_by_ordinal(module.as_ptr(), ord as u16).is_null() { + resolved_any = true; + break; + } + } + detours_exports_free(handle); + assert!(resolved_any, "at least one kernel32 ordinal should resolve"); + + // Guards. + assert!(detours_find_function_by_ordinal(ptr::null(), 1).is_null()); + assert!(detours_find_function_by_ordinal(invalid_utf8_cstr().as_ptr(), 1).is_null()); + // An absurd ordinal does not resolve. + assert!(detours_find_function_by_ordinal(module.as_ptr(), u16::MAX).is_null()); + } +} + +// --- scanning ---------------------------------------------------------------- + +#[test] +fn ffi_scan_range_finds_local_function_and_guards() { + let target = scan_me as *const u8; + let sig = signature_from(target, 16); + unsafe { + let found = detours_scan_range(target, 256, sig.as_ptr()); + assert_eq!(found, target); + + // Guards: null start, null pattern, invalid utf8, bad signature. + assert!(detours_scan_range(ptr::null(), 256, sig.as_ptr()).is_null()); + assert!(detours_scan_range(target, 256, ptr::null()).is_null()); + assert!(detours_scan_range(target, 256, invalid_utf8_cstr().as_ptr()).is_null()); + let bad = CString::new("48 ZZ").unwrap(); + assert!(detours_scan_range(target, 256, bad.as_ptr()).is_null()); + } +} + +#[test] +fn ffi_scan_module_finds_local_function_and_guards() { + let h_exe: HMODULE = unsafe { GetModuleHandleW(ptr::null()) }; + let target = scan_me as *const u8; + let sig = signature_from(target, 24); + unsafe { + let found = detours_scan_module(h_exe as *mut _, sig.as_ptr()); + assert_eq!(found, target); + + assert!(detours_scan_module(h_exe as *mut _, ptr::null()).is_null()); + assert!(detours_scan_module(h_exe as *mut _, invalid_utf8_cstr().as_ptr()).is_null()); + let bad = CString::new("48 ZZ").unwrap(); + assert!(detours_scan_module(h_exe as *mut _, bad.as_ptr()).is_null()); + } +} + +#[test] +fn ffi_scan_module_by_name_finds_kernel32_export_and_guards() { + let module = CString::new("kernel32.dll").unwrap(); + let target = neohook::find_function("kernel32.dll", "GetProcAddress").unwrap(); + let sig = signature_from(target, 12); + unsafe { + let found = detours_scan_module_by_name(module.as_ptr(), sig.as_ptr()); + assert_eq!(found, target); + + assert!(detours_scan_module_by_name(ptr::null(), sig.as_ptr()).is_null()); + assert!(detours_scan_module_by_name(module.as_ptr(), ptr::null()).is_null()); + assert!(detours_scan_module_by_name(invalid_utf8_cstr().as_ptr(), sig.as_ptr()).is_null()); + assert!( + detours_scan_module_by_name(module.as_ptr(), invalid_utf8_cstr().as_ptr()).is_null() + ); + let bad = CString::new("48 ZZ").unwrap(); + assert!(detours_scan_module_by_name(module.as_ptr(), bad.as_ptr()).is_null()); + } +} + +// --- attach_pattern / attach_export (transaction shims) ---------------------- + +#[test] +fn ffi_attach_pattern_end_to_end_and_guards() { + use std::sync::OnceLock; + type AddFn = extern "system" fn(i32, i32) -> i32; + static ORIG: OnceLock = OnceLock::new(); + + extern "system" fn detour(a: i32, b: i32) -> i32 { + ORIG.get().unwrap()(a, b) * 10 + } + + let mut buf = [0u16; 1024]; + let len = unsafe { + windows_sys::Win32::System::LibraryLoader::GetModuleFileNameW( + ptr::null_mut(), + buf.as_mut_ptr(), + buf.len() as u32, + ) + }; + let full = String::from_utf16_lossy(&buf[..len as usize]); + let module_name = full.rsplit(['\\', '/']).next().unwrap().to_string(); + let module = CString::new(module_name).unwrap(); + + let target = scan_me as *const u8; + let sig = signature_from(target, 24); + + assert_eq!(scan_me(2, 3), 12); // 2 + 3 + 7 + + unsafe { + let tx = detours_transaction_begin(); + detours_transaction_update_all_threads(tx); + let tramp = detours_transaction_attach_pattern( + tx, + module.as_ptr(), + sig.as_ptr(), + detour as *const u8, + ); + assert!(!tramp.is_null()); + ORIG.set(std::mem::transmute::<*mut u8, AddFn>(tramp)) + .unwrap(); + let handle = detours_transaction_commit(tx); + assert!(!handle.is_null()); + + assert_eq!(scan_me(2, 3), 120, "(2 + 3 + 7) * 10 via the patched body"); + + detours_handle_unhook_and_free(handle); + assert_eq!(scan_me(2, 3), 12, "restored"); + + // Guards. + assert!( + detours_transaction_attach_pattern( + ptr::null_mut(), + module.as_ptr(), + sig.as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + let tx2 = detours_transaction_begin(); + assert!( + detours_transaction_attach_pattern( + tx2, + invalid_utf8_cstr().as_ptr(), + sig.as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + assert!( + detours_transaction_attach_pattern( + tx2, + module.as_ptr(), + invalid_utf8_cstr().as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + detours_transaction_abort(tx2); + } +} + +#[test] +fn ffi_attach_export_guards() { + let module = CString::new("kernel32.dll").unwrap(); + let func = CString::new("GetProcAddress").unwrap(); + unsafe { + // Null tx / null strings. + assert!( + detours_transaction_attach_export( + ptr::null_mut(), + module.as_ptr(), + func.as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + + let tx = detours_transaction_begin(); + assert!( + detours_transaction_attach_export( + tx, + invalid_utf8_cstr().as_ptr(), + func.as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + assert!( + detours_transaction_attach_export( + tx, + module.as_ptr(), + invalid_utf8_cstr().as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + // Unknown export => null. + let missing = CString::new("NoSuchExport_zzz").unwrap(); + assert!( + detours_transaction_attach_export( + tx, + module.as_ptr(), + missing.as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + detours_transaction_abort(tx); + } +} + +// --- resolve ----------------------------------------------------------------- + +#[test] +fn ffi_resolve_call_and_rip_relative_guard_on_null() { + unsafe { + // Null / non-branch input resolves to null rather than crashing. + assert!(detours_resolve_call_target(ptr::null()).is_null()); + assert!(detours_resolve_rip_relative(ptr::null()).is_null()); + } +} + +#[test] +fn ffi_resolve_relative_decodes_a_synthetic_call() { + // E8 rel32: a 5-byte near call whose displacement is +0 lands right after it. + let code = [0xE8u8, 0x00, 0x00, 0x00, 0x00]; + unsafe { + let target = detours_resolve_relative(code.as_ptr(), 1, 5); + assert_eq!(target, code.as_ptr().add(5)); + + // And the decoder path resolves the same near call. + let decoded = detours_resolve_call_target(code.as_ptr()); + assert_eq!(decoded, code.as_ptr().add(5)); + } +} + +// --- VEH --------------------------------------------------------------------- + +#[test] +fn ffi_veh_install_redirects_and_unhooks() { + fn call() -> u32 { + let f = std::hint::black_box(veh_target as extern "system" fn() -> u32); + f() + } + assert_eq!(call(), 1234); + unsafe { + let hook = detours_veh_install(veh_target as *const u8, veh_detour as *const u8); + assert!(!hook.is_null()); + assert_eq!(call(), 9999, "VEH detour should win"); + assert_eq!(detours_veh_unhook(hook), 1); + assert_eq!(call(), 1234, "restored after unhook"); + + // Null guards. + assert!(detours_veh_install(ptr::null(), veh_detour as *const u8).is_null()); + assert_eq!(detours_veh_unhook(ptr::null_mut()), 0); + } +} + +// --- INT3 -------------------------------------------------------------------- + +#[test] +fn ffi_int3_install_redirects_and_unhooks() { + fn call() -> u32 { + let f = std::hint::black_box(int3_target as extern "system" fn() -> u32); + f() + } + assert_eq!(call(), 7); + unsafe { + let hook = detours_int3_install(int3_target as *const u8, int3_detour as *const u8); + assert!(!hook.is_null()); + assert_eq!(call(), 77, "INT3 detour should win"); + assert_eq!(detours_int3_unhook(hook), 1); + assert_eq!(call(), 7, "restored after unhook"); + + // Null guard on unhook. + assert_eq!(detours_int3_unhook(ptr::null_mut()), 0); + } +} + +// --- mid-function ------------------------------------------------------------ + +#[test] +fn ffi_midhook_install_runs_handler_and_unhooks() { + MID_RAN.store(false, Ordering::SeqCst); + let handler: MidHookHandler = mid_handler; + unsafe { + let hook = detours_midhook_install(mid_target as *const u8, handler); + assert!(!hook.is_null()); + + let _ = mid_target(41); + assert!(MID_RAN.load(Ordering::SeqCst), "handler must fire"); + + assert_eq!(detours_midhook_unhook(hook), 1); + + MID_RAN.store(false, Ordering::SeqCst); + let _ = mid_target(41); + assert!( + !MID_RAN.load(Ordering::SeqCst), + "handler must not fire after unhook" + ); + + // Null guards. + assert!(detours_midhook_install(ptr::null(), handler).is_null()); + assert_eq!(detours_midhook_unhook(ptr::null_mut()), 0); + } +} + +// --- delay / on-load --------------------------------------------------------- + +#[test] +fn ffi_delay_register_for_loaded_module_and_guards() { + // kernel32 is already loaded, so the delay hook installs immediately and + // reports active. We target a rarely-called export to avoid side effects. + let module = CString::new("kernel32.dll").unwrap(); + let func = CString::new("GetProcAddress").unwrap(); + unsafe { + let hook = detours_delay_register(module.as_ptr(), func.as_ptr(), scan_detour as *const u8); + // Registration against an already-loaded module should succeed. + assert!(!hook.is_null()); + assert_eq!(detours_delay_is_active(hook), 1, "loaded module => active"); + assert_eq!(detours_delay_unhook(hook), 1); + + // Guards. + assert!( + detours_delay_register(ptr::null(), func.as_ptr(), scan_detour as *const u8).is_null() + ); + assert!( + detours_delay_register(module.as_ptr(), ptr::null(), scan_detour as *const u8) + .is_null() + ); + assert!( + detours_delay_register( + invalid_utf8_cstr().as_ptr(), + func.as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + assert!( + detours_delay_register( + module.as_ptr(), + invalid_utf8_cstr().as_ptr(), + scan_detour as *const u8 + ) + .is_null() + ); + assert_eq!(detours_delay_is_active(ptr::null()), 0); + assert_eq!(detours_delay_unhook(ptr::null_mut()), 0); + } +} + +// --- transaction detach via FFI --------------------------------------------- + +#[test] +fn ffi_transaction_detach_guards() { + unsafe { + // Null tx / null handle are rejected. + assert_eq!( + detours_transaction_detach(ptr::null_mut(), ptr::null_mut(), 0), + 0 + ); + let tx = detours_transaction_begin(); + assert_eq!(detours_transaction_detach(tx, ptr::null_mut(), 0), 0); + detours_transaction_abort(tx); + } +} + +// --- set_enabled / is_enabled out-of-bounds ---------------------------------- + +#[test] +fn ffi_handle_enable_helpers_guard_oob_and_null() { + unsafe { + let tx = detours_transaction_begin(); + let tramp = detours_transaction_attach(tx, scan_me as *mut u8, scan_detour as *const u8); + assert!(!tramp.is_null()); + let handle = detours_transaction_commit(tx); + assert!(!handle.is_null()); + + // Valid round-trip. + assert_eq!(detours_handle_is_enabled(handle, 0), 1); + assert_eq!(detours_handle_set_enabled(handle, 0, 0), 1); + assert_eq!(detours_handle_is_enabled(handle, 0), 0); + assert_eq!(detours_handle_set_enabled(handle, 0, 1), 1); + + // Out-of-bounds index. + assert_eq!(detours_handle_set_enabled(handle, 99, 1), 0); + assert_eq!(detours_handle_is_enabled(handle, 99), 0); + + // Null handle. + assert_eq!(detours_handle_set_enabled(ptr::null_mut(), 0, 1), 0); + assert_eq!(detours_handle_is_enabled(ptr::null_mut(), 0), 0); + + detours_handle_unhook_and_free(handle); + } +}