[audit][medium] Malformed state-sync chunks are removed from pending before validation

[QuantumExplorer]

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

Summary

State sync removes a pending chunk before decoding and processing it.

Impact / failure scenario

A single malformed response can poison the session and prevent retrying the same chunk from another peer.

References

• grovedb/src/replication/state_sync_session.rs:102
• grovedb/src/replication/state_sync_session.rs:107
• grovedb/src/replication/state_sync_session.rs:109

Suggested fix

Remove a chunk from pending_chunks only after successful processing, or reinsert/rollback on error.

Suggested tests

Apply malformed bytes for a pending chunk, then valid bytes for the same chunk, and assert retry succeeds.

---

September 2026 audit addendum — S05

Audit group: S05. Classification: security. Provisional severity: medium.

Audit addendum for #695, based on provisional static review of snapshot 2fa0f133. The retained evidence confirms the same retry-state defect already described in this issue. This is an update to the existing report, not a new vulnerability or a claim that current develop has been checked for the same behavior.

The expected contract is that a response which fails validation does not consume the requested work needed for a subsequent valid replacement. The outer synchronization coordinator removes its pending request before decoding, verification and application finish. Later errors return without restoring that membership. The lower restorer retains its own expected commitment until a successful write, so the two layers disagree after rejection. The outer entry guard then prevents a valid retry within the same session.

This finding concerns synchronization availability and recovery, not forged-root acceptance. Unknown chunk identifiers are rejected before mutation, expected commitments are checked by the restorer, and rejected data is still reported as an error. An application can abandon the session and start again, which contains the persistent effect but requires recovery beyond the attempted per-chunk retry. How frequently that occurs, and whether it affects a live service, depends on integration policy that was not supplied.

Consume the outer pending request only after the response has been successfully validated and applied. Preserve coordinator and restorer expectations together on recoverable errors. Grouped responses also need a defined recovery contract so a later rejection does not make earlier progress ambiguous. Correcting request membership alone should not be presented as solving whole-snapshot transaction durability.

The saved issue inventory identifies #679 and #775 as separate early-commit concerns, and PR #840 proposes related synchronization changes. Its description does not establish that this retry invariant is repaired in the audited snapshot. Validation here was static only: no response reproduction, runtime retry sequence, network endpoint or deployment was exercised. Medium severity is retained with session restart documented as containment.

Validation to complete

• Check ordinary validation errors preserve expected request membership at both coordinator and restorer layers.
• Verify a subsequent ordinary valid response can satisfy the same outstanding request.
• Check grouped-response error handling leaves progress and completion reporting coherent.

Limits and existing work

• External retry/restart policy remains unknown.

Related tracking: issue #695 (open), issue #679 (open), issue #775 (open), PR #840 (open).

Scope: saved GroveDB worktree with revision context 2fa0f133877420a0d9c91ba7bc51b1775ab8c783. This report does not establish that current develop or any deployed application is affected. Focused runtime validation remains outstanding.

Audit source and canonical finding identifiers

Source status: snapshot-backed (git_worktree); plain source locations are used because this is not a sealed commit-only scan.

Audited revision context: 2fa0f133877420a0d9c91ba7bc51b1775ab8c783.

The findings were manually reconciled from a preserved scan bundle. The native scan ended before final completion; these are provisional source-review findings, not a completed native scan certification.

Canonical finding ID: csf_01109c4ee16a6d6b4063b230

Primary fingerprint: codex-security/v1:sha256:c0faf6d69d635ed80365ae2a6b3335fb491f252130f677466475fc4f223c4040

Canonical finding ID: csf_d02d9f2ec44574e7a4f4ee73

Primary fingerprint: codex-security/v1:sha256:b8ea83a4bac157d7bffaf120b68e050d39caadfb0ea1e714e277cd2fdfcd5f52

Source locations:

• Location (root_control): grovedb/src/replication/state_sync_session.rs:107
• Location (entrypoint): grovedb/src/replication/state_sync_session.rs:512
• Location (expected_control): merk/src/merk/restore.rs:145
• Location (root_control): grovedb/src/replication/state_sync_session.rs:107-111
• Location (outcome): grovedb/src/replication/state_sync_session.rs:119-130
• Location (propagation): grovedb/src/replication/state_sync_session.rs:512-516
• Location (root_control): grovedb/src/replication/state_sync_session.rs:102-129
• Location (entrypoint): grovedb/src/replication/state_sync_session.rs:512-516
• Location (root_control): grovedb/src/replication/state_sync_session.rs:102-111
• Location (root_control): grovedb/src/replication/state_sync_session.rs:107-109
• Location (entrypoint): grovedb/src/replication/state_sync_session.rs:511-521

[QuantumExplorer]

This is Claude, closing on behalf of the maintainer.

Superseded by the fail-closed design in #840 (6ec8fee). The chunk id is still removed from the pending set before decoding, but the consequence this issue worried about is gone: any decode or verification failure permanently fails the session, so is_sync_completed() reports false, every further apply_chunk and commit_session is refused, and in the default atomic mode nothing has reached disk. A malformed response can no longer leave a subtree unfinalized yet "complete". Retrying one chunk from a different peer inside the same session was deliberately not adopted; the caller abandons the session and restarts the restore (drive-abci wipes and re-syncs anyway).

Tests: the shared byzantine harness in replication_session_tests.rs asserts poisoning, refused commit and an untouched destination on every tamper case.