0xMiden / 0xMiden/protocol

PartialBlockchain::new panics on a PartialMmr that new_unchecked accepts

Aperta
#3,654 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Rust
Stelle
132
Fork
167
Merge medio
1g 23h
PR unite (30g)
110

Descrizione

`PartialBlockchain::new` unwraps `PartialMmr::open` with `.expect`, but the precondition it relies on is weaker than what `open` needs, so untrusted bytes can panic the process.

- **Symptom** — panic at `src/transaction/partial_blockchain.rs:74`, `block should not exceed chain length`.
- **Cause** — `new_unchecked` gates on `PartialMmr::is_tracked`, which only asserts a leaf entry exists at that position. `open` additionally needs every ancestor sibling on the leaf's authentication path; when one is absent it returns `Err`, and the `expect` turns that into a panic. The `// SAFETY:` comment at :69 names `new_unchecked`'s tracking check as the guarantee, and that check does not cover this case.
- **Repro** — build a `PartialMmr` tracking a leaf, remove one of that leaf's ancestor sibling nodes, pass it to `PartialBlockchain::new`.
- **Expected** — `PartialBlockchainError`, not a panic.
- **Impact** — reachable wherever a `PartialBlockchain` is reconstructed from bytes received from another party. Hit in `miden-client` through `ChainAnchor`, which carries a `PartialBlockchain` between clients; mitigated client-side in [0xMiden/rust-sdk#2432](https://github.com/0xMiden/rust-sdk/pull/2432) by opening every tracked block during deserialization, but the panic is still reachable by any other caller.

Source

`src/transaction/partial_blockchain.rs`:

```rust
// new(), :68-75
for (block_num, block) in partial_chain.blocks.iter() {
// SAFETY: new_unchecked returns an error if a block is not tracked in the MMR, so
// retrieving a proof here should succeed.
let proof = partial_chain
.mmr
.open(block_num.as_usize())
.expect("block should not exceed chain length")
.expect("block should be tracked in the partial MMR");

// new_unchecked(), :117-121
// Note that this only checks if a leaf exists at that position but it doesn't
// assert that it matches the block's commitment provided in the iterator.
if !mmr.is_tracked(block_num.as_usize()) {
return Err(PartialBlockchainError::untracked_block(block_num));
}
```

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.