fix(memory,scanner): guard region reads against TOCTOU faults

[tkhquang]

Summary

• TOCTOU sweep guards
  The per-region VirtualQuery gate proves a region readable only at gate time; a concurrent decommit or reprotect between the gate and the unguarded SIMD/Zydis reads faults the host. The per-region scan (scanner) and per-window scans (string-xref narrow + broad) now run inside a structured-exception guard on MSVC: a faulting region/window is skipped and counted instead of terminating the process, with the Nth-match / reference state rolled back so a skipped region contributes nothing. MinGW keeps the VirtualQuery gate as its only guard until the VEH-based handler lands.

• EXCEPTION_IN_PAGE_ERROR
  A single shared is_guarded_read_fault predicate now backs the four memory.cpp SEH filters and the new scanner/string-xref guards, so a file-backed page that fails to page in (0xC0000006) is swallowed everywhere instead of escaping to terminate the host.

• region-cache lows
  read_ptr_unsafe reads through memcpy (no misaligned-deref UB); invalidate_range_internal counts and debug-logs shards left unswept under contention; check_memory_permission falls back to a direct VirtualQuery when the cache reports zero shards, closing the init publication window.

Summary by CodeRabbit

Release Notes

• Documentation

  • Clarified memory scanning and read operation behaviors during fault conditions on MSVC and MinGW platforms.

• Bug Fixes

  • Improved resilience of scanning operations against concurrent memory access violations.
  • Fixed potential undefined behavior from misaligned pointer reads.

• Tests

  • Added tests validating scanner and cross-reference resilience during concurrent memory state changes.

[coderabbitai]

Warning

Review limit reached

@tkhquang, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 44 minutes and 29 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0de96ebf-9089-43e2-82fd-6a71769ea152

📥 Commits

Reviewing files that changed from the base of the PR and between 0598c02 and 4887b5c.

📒 Files selected for processing (2)

• tests/test_scanner.cpp
• tests/test_string_xref.cpp

📝 Walkthrough

Walkthrough

This PR hardens foreign-memory reads and pattern scanning against concurrent memory decommit/reprotect faults. It introduces a centralized SEH exception-filter predicate, applies it across Memory and Scanner modules, fixes misaligned pointer reads with std::memcpy, improves cache permission fallback, and refactors AOB and string-xref scanning to tolerate mid-sweep faults via MSVC structured-exception guarding. Tests validate the fault predicate, misaligned reads, cache fallback, and concurrent decommit resilience.

Changes

SEH Guarded Reading and Scanning Resilience

Layer / File(s)	Summary
SEH Fault-Filter Centralization src/memory_internal.hpp, src/memory.cpp, src/scanner.cpp, tests/test_memory.cpp	New internal header defines a constexpr is_guarded_read_fault() predicate that matches EXCEPTION_ACCESS_VIOLATION, STATUS_GUARD_PAGE_VIOLATION, and EXCEPTION_IN_PAGE_ERROR exception codes. Integrated into memory, scanner, and test modules with a new unit test validating the predicate for expected fault codes and rejection of unrelated exceptions.
Memory Module Hardening and Cache Resilience src/memory.cpp, include/DetourModKit/memory.hpp, tests/test_memory.cpp	SEH exception filters in seh_read_bytes, resolve_chain_guarded, and seh_read_chain_bytes now use the centralized fault predicate. read_ptr_unsafe replaces direct pointer dereferences with std::memcpy to avoid undefined behavior on misaligned reads across MSVC and non-MSVC paths. Cache invalidation tracks contended shards and logs when entries persist, and check_memory_permission falls back to direct VirtualQuery when cache is unavailable during init/shutdown. Tests cover misaligned-pointer round-trips, cache fallback correctness, and expanded fault-predicate coverage.
AOB Scanner Guarded Region Scanning src/scanner.cpp, src/scanner_internal.hpp, README.md, tests/test_scanner.cpp	Introduces scan_region_guarded and scan_region_for_match internal helpers that wrap pattern searches with MSVC SEH fault guarding (or direct execution on non-MSVC) to survive concurrent decommit/reprotect mid-sweep. Integrates guarded scanning into scan_regions_filtered, tracks faulted regions, and logs diagnostics. Documentation clarifies that ExecutableWindow guarantees readability only at gate time and that callers must still guard against concurrent faults. README specifies that pure-execute pages are skipped and concurrent decommit/reprotect is tolerated. Includes MSVC-only concurrency regression test.
String Xref Guarded Window Scanning src/string_xref.cpp, AGENTS.md, tests/test_string_xref.cpp	Refactors Phase 2 string-xref scanning (narrow and broad) into bodies and guarded wrappers that swallow guarded faults per window and log which windows faulted. Both scan phases track faulted windows and return success only when exactly one reference site is found across non-faulted windows (fail-closed on ambiguity or zero matches). Updates module includes for logger support. AGENTS.md documents the raw Scanner::find_pattern overload behavior (no page filtering, caller-guaranteed memory) and the shared guarded-read-fault predicate across guarded reads. Includes MSVC-only concurrency regression test with RAII memory management.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• tkhquang/DetourModKit#87: Updates SEH-guarded pointer-chain read paths that directly overlap with this PR's centralized guarded-read exception-filter predicate applied to seh_resolve_chain/seh_read_chain_bytes.
• tkhquang/DetourModKit#103: Modifies the same src/memory.cpp seh_read_bytes and src/scanner.cpp AOB scanning hot paths with related guarded/reader behavior updates.
• tkhquang/DetourModKit#91: Hardens scanner-path reads by switching to Memory::seh_read, directly connected to this PR's core SEH-guarded read mechanism via the shared fault predicate.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding guards to region reads to protect against TOCTOU (time-of-check-time-of-use) faults involving memory that may be decommitted/reprotected concurrently.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

tests/test_string_xref.cpp (1)

817-825: 💤 Low value

Consider explicit capture list for the thread lambda.

The coding guidelines specify explicit capture lists for lambdas passed to threads. While the current [&] capture is safe (the jthread joins before captures go out of scope), explicit captures make the dependencies clear.

Suggested change

     std::jthread toggler(
-        [&](std::stop_token stop_token)
+        [decommit_page, page](std::stop_token stop_token)
         {
             while (!stop_token.stop_requested())
             {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_string_xref.cpp` around lines 817 - 825, Replace the generic [&]
capture in the std::jthread "toggler" lambda with an explicit capture list for
the variables it uses (e.g., decommit_page and page) so dependencies are clear;
update the lambda capture to explicitly capture &decommit_page and &page and
leave the stop_token as the function parameter used inside, ensuring VirtualFree
and VirtualAlloc calls still reference the same captured variables.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/string_xref.cpp`:
- Around line 219-240: The loop in phase 2 (using
Scanner::detail::collect_executable_windows and scan_window_narrow_guarded)
currently returns first_site when found_count == 1 even if faulted_windows > 0,
which hides indeterminate results; change the logic in the block that logs
faulted_windows (and the analogous block at 364-380) so that if faulted_windows
> 0 the function does not return success but instead returns an indeterminate
error (introduce or use a StringXrefError value for "IndeterminateReference" or
adjust find_string_xref's return type to convey this error), i.e., after logging
via log_faulted_windows, if faulted_windows > 0 return the new indeterminate
StringXrefError rather than first_site, otherwise behave as before (return 0 for
none, first_site for a guaranteed single hit).

In `@tests/test_memory.cpp`:
- Around line 2119-2185: The new TESTs should be converted to use the existing
MemoryTest fixture and must restore global cache state: move the body of
MemoryGuardedReadFault.AcceptsForeignReadFaultsAndRejectsOthers and
MemoryUninitializedCache.PermissionChecksFallBackToDirectQuery into TEST_F
suites (or add them to the MemoryTest class), and in
MemoryTest::SetUp()/TearDown() ensure Memory::shutdown_cache() and the cache
re-initialization calls are balanced so the global cache state is always
restored after each test; specifically replace standalone TEST(...) definitions
with TEST_F(MemoryTest, ...) and add calls in TearDown() to reinitialize or
restore the Memory cache (or remove the shutdown call from the test and instead
perform the shutdown in SetUp()/TearDown() with proper backup/restore) so tests
are order-independent and follow the project fixture pattern.

---

Nitpick comments:
In `@tests/test_string_xref.cpp`:
- Around line 817-825: Replace the generic [&] capture in the std::jthread
"toggler" lambda with an explicit capture list for the variables it uses (e.g.,
decommit_page and page) so dependencies are clear; update the lambda capture to
explicitly capture &decommit_page and &page and leave the stop_token as the
function parameter used inside, ensuring VirtualFree and VirtualAlloc calls
still reference the same captured variables.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 97ef8bac-f999-48d1-b93b-6aa9b6ac9065

📥 Commits

Reviewing files that changed from the base of the PR and between 8eef23c and 0598c02.

📒 Files selected for processing (11)

• AGENTS.md
• README.md
• include/DetourModKit/memory.hpp
• src/memory.cpp
• src/memory_internal.hpp
• src/scanner.cpp
• src/scanner_internal.hpp
• src/string_xref.cpp
• tests/test_memory.cpp
• tests/test_scanner.cpp
• tests/test_string_xref.cpp