Smart Contract Diff Audit L-26: P2IDE Notes With Out-Of-Range Reclaim Heights Are Spendable On Chain But Reported As Never Consumable
- Lingua principale
- Rust
- Stelle
- 132
- Fork
- 167
- Merge medio
- 1g 23h
- PR unite (30g)
- 110
Descrizione
P2IDE notes hold the timelock and reclaim block heights as raw field elements in note storage, and the note script compares them against the reference block number with the field-element `lte` operation in [`assert_unlocked`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/asm/standards/notes/p2ide.masm#L49-L56) and [`reclaim_note`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/asm/standards/notes/p2ide.masm#L76-L95), without any `u32` bounds check. Storage values are otherwise unconstrained, since [`NoteStorage::new`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/storage.rs#L37-L45) bounds only the number of items and the kernel constrains only that count and the storage commitment. A sender can therefore assemble the storage vector by hand through public Rust APIs and produce a P2IDE note whose reclaim height exceeds `u32::MAX`, even though the typed constructor accepts only a `BlockNumber`.
Such a note stays consumable by its target on chain, because the target branch in [`main`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/asm/standards/notes/p2ide.masm#L158-L175) never reads the reclaim height. Rust decoding rejects any non-zero height whose canonical value does not fit into a `u32` in [`decode_block_height`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/p2ide.rs#L371-L385), and [`P2ideNoteStorage::try_from`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/p2ide.rs#L325-L366) runs before any consumability logic, so [`is_consumable`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/mod.rs#L196-L209) converts the decoding error into a `NeverConsumable` status that [`can_consume`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-tx/src/executor/notes_checker.rs#L216-L229) returns without ever executing the note. A spendable note is thus hidden from its target by standard tooling, while the batch checking path, which executes the notes instead of relying on this classification, remains unaffected. The divergence is one-sided, as an out-of-range timelock height is rejected by both the script and the decoder.
Consider aligning the note script and the Rust decoder on a single height domain. One option is to enforce `u32` bounds in the script before the height comparisons, which makes the script reject such notes and turns a currently spendable note into an unspendable one. The other is to handle out-of-range heights explicitly on the Rust side so that classification follows the on-chain semantics. Whichever direction matches the intended design, consider documenting the height domain as an invariant of P2IDE note construction and covering it with tests.
---
_Copied verbatim from finding [L-26](https://audits.openzeppelin.com/miden/miden-01-07-smart-contract-diff-audit-nfts/issues/p2ide-notes-with-out-of-range-reclaim-heights-are-spendable-on-chain-but-reported-as-never-consumable-bb806aae) (low severity) of the OpenZeppelin [smart contract diff audit (NFTs)](https://audits.openzeppelin.com/miden/miden-01-07-smart-contract-diff-audit-nfts). The audit was performed against commit `8411bf093bde25285708faac152b6d7269009617`._
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.