fix(scanner): repair prologue-fallback cascade recovery (#112)

Author: tkhquang

.clang-format

@@ -12,6 +12,13 @@ Standard: Latest
 # and a line whose excess is inside one literal is left alone.
 ColumnLimit: 120
 ReflowComments: Always
+# ReflowComments: Always otherwise re-wraps the deep hang-indented continuation lines of a /** */ Doxygen block and
+# mangles them (collapsing the alignment under the @tag, occasionally emitting a stray "* *"). CommentPragmas marks
+# comment lines that must never be reflowed: matching 3+ leading content spaces (the hang indent under a Doxygen tag)
+# protects every continuation line, which inhibits reflow of the whole block, so hand-wrapped doc-blocks are left intact.
+# Plain // implementation comments carry no such indent and are still reflowed to the column limit. The IWYU default
+# pattern is preserved.
+CommentPragmas: '^ IWYU pragma:|^ {3,}'
 BreakStringLiterals: false
 IndentWidth: 4
 TabWidth: 4

AGENTS.md

@@ -221,9 +221,9 @@ m_pending_messages.fetch_add(1, std::memory_order_acq_rel);
 
 ### Formatting and tooling
 
-C++ formatting is codified in the root `.clang-format` (LLVM base, Allman braces for functions/classes, 4-space indent, east-side pointers, includes never reordered). Run clang-format over the changed `*.cpp`/`*.hpp` before committing. CI runs an advisory check in `.github/workflows/quality.yml` (clang-format 20, the version pinned there) over the tracked project sources; submodules under `external/` are never formatted. The hard column limit is 120 (`ColumnLimit: 120`): code and comments must fit within it, and the formatter reflows comment text that runs past it (`ReflowComments: Always`). String literals are the one sanctioned exception: `BreakStringLiterals: false` keeps long log and error messages as single greppable literals, so a line whose excess sits inside one literal is left alone rather than split. When a message line still exceeds 120 columns, split the literal by hand at sentence or clause boundaries using adjacent string-literal concatenation (the compiler fuses the fragments back into one literal at compile time): keep the distinctive lead phrase whole in its own fragment so it stays greppable, and mind the space at each seam -- a missing seam space silently corrupts the message. Do not work around the limit with trailing comments -- keep a member's documentation on the line(s) above it (per the comment conventions) instead of letting a trailing comment push the line out. The comment-marker rules above are guarded by an advisory CI step, `scripts/check_comment_style.py` (no trailing `///<`, no multi-line `///`, no block tag on a `///` line).
+C++ formatting is codified in the root `.clang-format` (LLVM base, Allman braces for functions/classes, 4-space indent, east-side pointers, includes never reordered). Run clang-format over the changed `*.cpp`/`*.hpp` before committing. CI runs an advisory check in `.github/workflows/quality.yml` (clang-format 20, the version pinned there) over the tracked project sources; submodules under `external/` are never formatted. The hard column limit is 120 (`ColumnLimit: 120`): code and comments must fit within it, and the formatter reflows plain `//` comment text that runs past it (`ReflowComments: Always`), while `/** */` Doxygen doc-blocks are exempted from reflow via `CommentPragmas` (see the next paragraph). String literals are the one sanctioned exception: `BreakStringLiterals: false` keeps long log and error messages as single greppable literals, so a line whose excess sits inside one literal is left alone rather than split. When a message line still exceeds 120 columns, split the literal by hand at sentence or clause boundaries using adjacent string-literal concatenation (the compiler fuses the fragments back into one literal at compile time): keep the distinctive lead phrase whole in its own fragment so it stays greppable, and mind the space at each seam -- a missing seam space silently corrupts the message. Do not work around the limit with trailing comments -- keep a member's documentation on the line(s) above it (per the comment conventions) instead of letting a trailing comment push the line out. The comment-marker rules above are guarded by an advisory CI step, `scripts/check_comment_style.py` (no trailing `///<`, no multi-line `///`, no block tag on a `///` line).
 
-`ReflowComments: Always` only re-wraps comment text that *overflows* 120; it does not re-fill a block wrapped well short of the limit, nor re-join a Doxygen block hand-wrapped with an off-style decoration (a two-space hang after the `*`, or `@brief` placed inline on the `/**` line), and the clang-format check is advisory. Reflow such blocks by hand to fill the column, changing only line breaks (never wording): keep CRLF line endings (a text-mode read/write that flips them to LF rewrites the whole file); keep each `@tag` on the first line of its paragraph with continuations hanging-aligned under the tag content (the `@note`/`@return` style used throughout `hook_manager.hpp`); leave `@param`/`@tparam` description-aligned and single-line `/** ... */` untouched. Verify with `awk 'length>120'` (empty) and confirm the file's comment text is unchanged token-for-token.
+`ReflowComments: Always` re-wraps plain `//` comment text that overflows 120, but `CommentPragmas: '^ IWYU pragma:|^ {3,}'` exempts `/** */` Doxygen doc-blocks from reflow: it marks any comment line indented three or more spaces -- the hang under a Doxygen `@tag` -- as un-reflowable, and a single protected continuation line inhibits reflow of the whole block. So the formatter never re-wraps, re-fills, or mangles a doc-block; doc-blocks are hand-maintained (without the pragma, `ReflowComments: Always` collapses the hang-alignment of a re-wrapped block and can emit a stray `* *`). The clang-format check is advisory regardless. Reflow doc-blocks by hand to fill the column, changing only line breaks (never wording): keep CRLF line endings (a text-mode read/write that flips them to LF rewrites the whole file); keep each `@tag` on the first line of its paragraph with continuations hanging-aligned under the tag content (the `@note`/`@return` style used throughout `hook_manager.hpp`); leave `@param`/`@tparam` description-aligned and single-line `/** ... */` untouched. Because the formatter will not wrap a doc-block for you, verify length by hand with `awk 'length>120'` (empty) and confirm the comment text is unchanged token-for-token.
 
 Markdown files (`*.md`) are **not** hard-wrapped at 80 columns. Write one logical line per paragraph, list item, and blockquote line and let editors soft-wrap; do not insert manual line breaks mid-paragraph. Fenced code blocks, tables, and any line indented four or more spaces (indented code, nested list sub-paragraphs) are kept verbatim. As in code, use `--` rather than an em-dash or en-dash.
 

docs/misc/aob-signatures.md

@@ -251,7 +251,7 @@ if (range)
 }
 ```
 
-One range scan covers both `.text` and `.rdata` / `.data` candidates, so there is no `ScannerKind` parameter. An invalid range returns `ResolveError::InvalidRange` and never falls back to a whole-process scan (which would re-introduce the cross-module shadowing the overload exists to prevent). `resolve_cascade_in_module_with_prologue_fallback` is the module-scoped counterpart of the prologue-fallback resolver: the rewritten near-JMP must be found inside the range, but its jump destination may still point at a sibling mod's trampoline outside the module, so the destination is validated only against "lies in some loaded module".
+One range scan covers both `.text` and `.rdata` / `.data` candidates, so there is no `ScannerKind` parameter. An invalid range returns `ResolveError::InvalidRange` and never falls back to a whole-process scan (which would re-introduce the cross-module shadowing the overload exists to prevent). `resolve_cascade_in_module_with_prologue_fallback` is the module-scoped counterpart of the prologue-fallback resolver: the rewritten near-JMP must be found inside the range, but its jump destination may still point at a sibling mod's trampoline outside the module, so the destination is validated only as a plausible pointer on a committed, execute-readable page -- not constrained to the scanned range or to any loaded module, because SafetyHook and relay-trampoline detours can allocate code outside every image.
 
 > Use `resolve_cascade_in_module` only for a single contiguous mapped image. For packed or protected targets whose code is unpacked into separate `VirtualAlloc` regions, use the whole-process `resolve_cascade` (which `scan_executable_regions` walks for exactly that reason).
 
@@ -502,7 +502,7 @@ DetourModKit::Logger::get_instance().info(
 
 `resolve_cascade` is fine when the target function still looks the way your signature remembers it. It stops working as soon as another mod, loaded earlier in the process, inline-hooks the same function: SafetyHook, MinHook, and most hand-rolled detour libraries overwrite the first five bytes with a near-JMP (`E9 ?? ?? ?? ??`) to their trampoline. Your Direct-mode candidate that matches on a prologue byte sequence now sees `E9` instead of `48 89 5C 24 ...`, and the scan misses even though the function itself is still present.
 
-`resolve_cascade_with_prologue_fallback` handles that exact scenario. On the happy path it is identical to `resolve_cascade`. If every candidate misses, it walks the list again and, for each Direct-mode candidate, rebuilds the pattern with the first five tokens replaced by `E9 ?? ?? ?? ??` while preserving the literal tail. It scans with the rewritten pattern and then applies two guardrails before accepting a hit: the rewritten pattern must resolve to exactly one location across all executable regions (a unique JMP into the sibling mod's trampoline, not an arbitrary near-JMP that happens to share a tail shape), and the `E9` displacement must land inside a loaded module (so the jump target is a real function, not garbage). RipRelative candidates are skipped in the fallback phase since they target instructions deeper than the 5-byte prologue and are unaffected by the overwrite.
+`resolve_cascade_with_prologue_fallback` handles that exact scenario. On the happy path it is identical to `resolve_cascade`. If every candidate misses, it walks the list again and, for each Direct-mode candidate, rebuilds the pattern with the first five tokens replaced by `E9 ?? ?? ?? ??` while preserving the literal tail. It scans with the rewritten pattern and then applies two guardrails before accepting a hit: the rewritten pattern must resolve to exactly one location across all executable regions (a unique JMP into the sibling mod's trampoline, not an arbitrary near-JMP that happens to share a tail shape), and the `E9` displacement must resolve to a committed, execute-readable page (so the jump target is real code, not unmapped or data-only garbage). The destination is deliberately *not* required to lie inside a loaded module: SafetyHook trampolines and relay-style detours can live outside every image, so an in-module requirement would reject the precise recovery this path exists for. The recovered address honors the candidate's `|` anchor offset exactly as the direct pass would, so a `|`-anchored Direct candidate resolves to the same byte whether it matched directly or through the fallback. RipRelative candidates are skipped in the fallback phase since they target instructions deeper than the 5-byte prologue and are unaffected by the overwrite.
 
 ```cpp
 const auto hit = sc::resolve_cascade_with_prologue_fallback(

include/DetourModKit/scanner.hpp

@@ -477,8 +477,8 @@ namespace DetourModKit
          * @brief Cascade resolver with inline-hooked-prologue recovery.
          * @details Equivalent to resolve_cascade() on the happy path. If every candidate fails, rebuilds each
          *          Direct-mode candidate's pattern with the first 5 bytes replaced by `E9 ?? ?? ?? ??` (the near-JMP
-         *          signature that SafetyHook and MinHook write when another mod already hooked the target) and retries.
-         *          If the recovery path succeeds the log line calls this out explicitly.
+         *          shape used by SafetyHook and other rel32 inline detours) and retries. If the recovery path succeeds
+         *          the log line calls this out explicitly.
          *
          *          RipRelative candidates are skipped in the fallback phase since they target instructions deeper than
          *          the 5-byte prologue and are unaffected by the overwrite.
@@ -541,11 +541,11 @@ namespace DetourModKit
          *          near-JMP overwrites a code prologue, never data, so a match in .rdata / .data would be a false
          *          positive (the data-capable readable sweep is only used for the primary candidate pass).
          *
-         *          The rebuilt near-JMP must be FOUND inside @p range, but its jump destination is intentionally NOT
-         *          constrained to @p range. When a sibling mod inline-hooks the target, its E9 jumps to a trampoline
-         *          the sibling allocated outside this image, so the destination is validated only against "lies in some
-         *          loaded module" (which still rejects a jump into unmapped or data-only memory). Requiring the
-         *          destination in-range would reject the very recovery this path exists to perform.
+         *          The rebuilt near-JMP must be found inside @p range, but its jump destination is intentionally not
+         *          constrained to @p range or to any loaded module. When a sibling mod inline-hooks the target, its E9
+         *          usually jumps to a VirtualAlloc'd trampoline outside every image, so the destination is validated as
+         *          a plausible pointer on a committed, execute-readable page instead. This still rejects jumps into
+         *          unmapped or data-only memory without rejecting the recovery this path exists to perform.
          *
          * @param candidates Ordered candidates.
          * @param label Human-readable identifier used in log messages.

src/scanner.cpp

@@ -868,6 +868,21 @@ std::vector<Scanner::detail::ExecutableWindow> Scanner::detail::collect_executab
     return windows;
 }
 
+// Single-address sibling of the executable-page gate scan_regions_filtered applies per region. One VirtualQuery,
+// matched against the identical mask (MEM_COMMIT, EXECUTABLE_PAGE_FLAGS, not PAGE_GUARD / PAGE_NOACCESS), so the
+// prologue-recovery fallback can vet a decoded E9 destination without re-deriving the Windows page masks or
+// constraining it to a loaded module (a sibling mod's trampoline is VirtualAlloc'd outside every image).
+bool Scanner::detail::is_executable_address(std::uintptr_t address) noexcept
+{
+    MEMORY_BASIC_INFORMATION mbi{};
+    if (VirtualQuery(reinterpret_cast<LPCVOID>(address), &mbi, sizeof(mbi)) == 0)
+    {
+        return false;
+    }
+    const bool protection_unsafe = (mbi.Protect & (PAGE_GUARD | PAGE_NOACCESS)) != 0;
+    return mbi.State == MEM_COMMIT && (mbi.Protect & EXECUTABLE_PAGE_FLAGS) != 0 && !protection_unsafe;
+}
+
 const std::byte *DetourModKit::Scanner::scan_executable_regions(const CompiledPattern &pattern, size_t occurrence)
 {
     if (pattern.empty() || occurrence == 0)