fix: Refuse to walk an invalid SHAMap in getMissingNodes

[bthomee]

Part 7/16 of a stack. Base: part 6 (bthomee/shamap-invalid-06-refuse-immutable).

High Level Overview of Change

SHAMap::getMissingNodes() now refuses to walk past a position only a leaf may occupy, instead of
letting SHAMapNodeID::getChildNodeID() throw std::logic_error out of a job-queue worker thread and
crash the process via std::terminate(). This position is reachable remotely, without any node ever
passing through addKnownNode()'s own guard (added in part 5), via an unsolicited liAS_NODE reply
seeding the fetch pack — a conditioned but real remote denial-of-service. Also adds a per-sanitizer
XRPL_ASAN/XRPL_TSAN/XRPL_UBSAN compile define in cmake/XrplSanitizers.cmake, alongside the
existing dot-joined SANITIZERS string, so test code can detect one specific sanitizer directly.

Context of Change

A walk that reaches a position only a leaf may occupy now marks the map Invalid and abandons the
descent. Reaching that position was otherwise fatal: SHAMapNodeID::getChildNodeID() throws
std::logic_error past kLeafDepth, and there is no try/catch around getMissingNodes() in
InboundLedger::trigger(), around job.doJob() in JobQueue, or in Workers::Worker::run(), so the
exception leaves the thread function and reaches std::terminate(). It is reachable without any node
passing through addKnownNode(): InboundLedgers::gotStaleData() stores any parseable node from an
unsolicited liAS_NODE reply into the fetch pack keyed by its own hash, with no relatedness check, and
getFetchPack() re-verifies only that hash, so such a node canonicalizes into the tree and the walk
descends onto it. Steering which hash a node acquires needs it to have no trusted validations, which
makes this a conditioned remote denial of service rather than a single-packet one.

As in addKnownNode(), the depth test precedes the full-below cache lookup, for the same reason: that
cache is keyed by node hash and shared across maps, and a hash does not cover depth, so a hit would
carry the whole branch past the guard. Four further properties of isValid() bound what the walk does
once the verdict lands: it short-circuits on entry rather than re-deriving a verdict already reached; it
breaks out of the descent but falls through to the deferred-read drain; it discards whatever was
collected; and it re-tests before clearSynching(), since another thread's addKnownNode() can write
the verdict after the loop's own test.

DeepChain gains withDecoys(): the same chain, but with a second and unresolvable child at every
level, so a backed map's descendAsync() posts a real asynchronous read at every level — needed by the
ThreadSanitizer case in this same commit.

Seven cases cover this: five drive the walk through a ChainFilter (standing in for a fetch pack); a
sixth pins that addRootNode() cannot clear the synching flag on an invalid map; the seventh races a
walk against setImmutable() under ThreadSanitizer. That last case needed a way to ask the compiler
whether TSAN is active from within a test file — the XRPL_TSAN compile define this branch adds to
cmake/XrplSanitizers.cmake alongside XRPL_ASAN/XRPL_UBSAN is what it now checks, rather than a
local GCC/Clang feature-macro dance.

API Impact

• libxrpl change (any change that may affect libxrpl or dependents of libxrpl)

[Copilot]

Pull request overview

This PR hardens SHAMap::getMissingNodes() against invalid tree shapes by refusing to descend into an inner node at a depth only leaves may occupy, preventing a std::logic_error from escaping worker threads and terminating the process. It also adds per-sanitizer compile defines so tests can reliably detect the active sanitizer.

Changes:

• Add a depth guard in the missing-node walk that marks the map Invalid and safely aborts further descent while still draining deferred reads.
• Expand the SHAMap sync test suite with targeted cases (including a TSAN-only concurrency test) and extend DeepChain to generate “decoy” children for async-read coverage.
• Add XRPL_ASAN / XRPL_TSAN / XRPL_UBSAN compile definitions in cmake/XrplSanitizers.cmake.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/tests/libxrpl/shamap/SHAMapSync.cpp	Adds ChainFilter and multiple tests covering invalid-depth handling, deferred-read draining, cache interaction, and TSAN concurrency behavior.
src/tests/libxrpl/shamap/DeepChain.h	Adds DeepChain::withDecoys() to force async reads during descent for sanitizer-sensitive test coverage.
src/libxrpl/shamap/SHAMapSync.cpp	Implements the depth guard during traversal and adds invalid-map short-circuiting + safe draining semantics.
include/xrpl/shamap/SHAMap.h	Updates public API documentation for the new “empty result may mean Invalid” behavior and state transitions.
cmake/XrplSanitizers.cmake	Defines per-sanitizer preprocessor macros alongside the existing SANITIZERS string.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[xrplf-ai-reviewer]

This is a well-scoped, carefully documented fix that adds a depth guard to SHAMap::getMissingNodes()/gmnProcessNodes() to prevent walking past kLeafDepth (which previously threw std::logic_error and crashed the process via std::terminate()), plus a matching XRPL_ASAN/XRPL_TSAN/XRPL_UBSAN compile-define addition and seven new targeted tests. The guard is correctly ordered ahead of the full-below cache lookup (mirroring addKnownNode's existing rationale), the invalidation path falls through to drain in-flight deferred reads before discarding results, and isValid() is re-checked after the loop to avoid racing clearSynching() against a concurrent invalidation. I did not find a correctness or security bug in the added code; the only note is a minor, low-confidence observation about log volume on repeated calls to an already-invalid map.

[codecov]

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/libxrpl/shamap/SHAMapSync.cpp	94.4%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!