[audit][low] Lazy-loaded Merk references do not validate fetched node metadata

[QuantumExplorer]

Created from a Codex audit of grovedb. No code changes were made as part of the audit.

Summary

When a Link::Reference is loaded, the fetched node is trusted. The key check is debug-only and hash/aggregate/height metadata are not validated against the reference.

Impact / failure scenario

Corrupt storage or a custom Fetch implementation can return a stale or wrong node; release builds can continue with inconsistent link metadata and loaded tree contents.

References

• merk/src/tree/mod.rs:1497
• merk/src/tree/walk/mod.rs:90
• merk/src/merk/source.rs:51

Suggested fix

Validate fetched key, hash_for_link(tree_type), aggregate data, and child heights before converting a reference link into a loaded node.

Suggested tests

Corrupt a referenced child record after commit and assert get/prove fails at load time.

[QuantumExplorer]

Downgrading high → low after adversarial verification. The unvalidated lazy-load path is real, but it only fires on local-fault scenarios: MerkSource::fetch reads from the same storage context/snapshot the link metadata was committed in, so in normal operation (block execution, proof generation, AVL rebalancing) the fetched node always matches its link. There is no remote vector — state-sync restore verifies chunks against the consensus-trusted root and rewrites child heights before the Merk is returned, and proof verification never uses these functions. The validation added in #736 remains worth merging as hardening against storage corruption and Fetch-implementation bugs.

[QuantumExplorer]

This is Claude, triaging on behalf of @QuantumExplorer.

Closing as outside the trust boundary / by design. The residual claim is accurate — TreeNode::load (merk/src/tree/mod.rs:1570-1614) installs a fetched node with the parent link's hash, child_heights and aggregate_data copied verbatim and verifies none of them, and Walker::detach (merk/src/tree/walk/mod.rs:80-95) discards the link before fetching, so the apply path has no check at all. But:

• The trigger is corrupt local storage or a hostile Fetch impl. Storage is inside grovedb's trust boundary, and Fetch has exactly two in-tree implementations (MerkSource, PanicSource). No untrusted network input reaches load.
• Proofs generated over a corrupt child fail verifier-side regardless, because the verifier recomputes kv_hash from the transmitted value.
• GroveDB already ships explicit integrity checkers for this threat: Merk::verify and GroveDb::verify_grovedb.

There is also a strong argument against fixing it: validating hash_for_link(tree_type) and aggregate data on every link load adds hash_node_calls to OperationCost on the hot read path. Cost changes are replay-critical — Platform replays historical blocks using the estimate as an admission bound — so this would be a consensus-grade change bought to defend against a threat already outside the model.

Becomes real if a Fetch implementation is ever exposed to untrusted data (for example a network-backed source).

One correction worth recording: the issue says "the key check is debug-only". It is worse than that and simultaneously less meaningful — the debug_assert_eq!(tree.key(), link.key()) at merk/src/tree/mod.rs:1606 is tautological and can never fire, because TreeNode::decode overwrites the decoded key with the lookup key (merk/src/tree/encoding.rs:136-137). So there is no key check in any build profile — but there is also no key confusion to catch, since the fetch is keyed by the link. That dead assertion currently reads as a safety check while checking nothing; deleting or correcting it is a worthwhile one-liner.

Rationale being added to docs/audit-non-issues.md.