Skip to content

next_handle is adopted unvalidated from the superblock and incremented unchecked — the same defect as SECURITY-SWEEP-6, one field over #152

Description

@Xof

Found by the adversarial review of #151 (the txn_counter bound for #108 / SECURITY-SWEEP-6), which asked what else in the same code path was unbounded.

The defect

next_handle is the last unvalidated superblock scalar that feeds arithmetic:

  • src/transaction/recovery.rs:502 adopts it verbatim — next_handle: sb.next_handle — and Superblock::validate does not bound it.
  • src/transaction/staging.rs:325 then does self.current_roots.next_handle += 1;unchecked, not even the checked_add that I119 mandated for txn_counter.

Why it matters

Threat model is the usual one: the attacker controls the file bytes and re-stamps the XXH3 checksum for free.

Forge bytes 40..48 of a plaintext superblock to u64::MAX. The first allocate mints handle u64::MAX, then:

  • debug builds — panics on overflow, bypassing the poison model exactly as the txn_counter expect did, and reaching Python as a PanicException rather than a mapped error class;
  • release builds — wraps to 0, which is the reserved "no handle" sentinel (handle_table.rs:48), and then collides with live handles.

Encrypted databases are not exposed: next_handle lives inside the AEAD-sealed superblock body. Plaintext databases are squarely in the stated threat model.

Aggravating factor

docs/reviews/review-20260729-183138.md:251 dismisses a handle-table path as "unreachable today — next_handle … is only ever += 1". That is the exact faulty premise #151 corrected for txn_counter: the value is not only produced by our own increments, it is also read from the file. The forge makes that deep-tree path reachable in a single allocate call.

Direction of a fix

Mirror what #151 did for txn_counter, on both sides:

  1. Read side — bound next_handle in Superblock::validate with a SuperblockDefect, so a forged slot loses to a healthy sibling rather than failing the whole open.
  2. Write side — make staging.rs:325 refuse rather than wrap, so the engine never persists a superblock it would refuse to read back. fix: bound txn_counter at the trust boundary (#108) #151's superblock::next_txn_counter helper is the pattern.

Also worth correcting the review-doc sentence at review-20260729-183138.md:251, since it is the reasoning a future maintainer would trust.

Metadata

Metadata

Assignees

No one assigned

    Labels

    severity:designWrong shape: bad abstraction, unenforced invariant, doc contradicts codetype:securityTrust boundary, unsafe, hostile-input handling

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions