Smart Contract Diff Audit L-11: Off-Chain P2IDE Consumability Analysis Diverges From Note Script Reclaim-Height Semantics
- 主要語言
- Rust
- 星號
- 132
- 分支
- 167
- 平均合併
- 1 天 23 小時
- 30 天內合併 PR
- 110
描述
`P2IDE` notes store the timelock and reclaim block heights as raw field elements, 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). There is no `u32` bounds check, and a stored reclaim height of zero is treated as "reclaim permanently disabled," not "reclaimable since genesis." Storage values are otherwise unconstrained: [`NoteStorage::new`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/storage.rs#L37-L45) bounds only the item count, so a sender can assemble a reclaim height by hand through public Rust APIs, whether that height is exactly zero or larger than `u32::MAX`, even though the typed constructor accepts only a `BlockNumber`. Off-chain, both values break the same consumability analysis, in opposite directions and through different mechanisms.
An out-of-range reclaim height hides a note the target can genuinely consume. The target branch of [`main`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/asm/standards/notes/p2ide.masm#L158-L175) never reads the reclaim height at all, so the note stays consumable on chain regardless of that field's value. However, [`decode_block_height`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/p2ide.rs#L371-L385) rejects any non-zero height that does not fit a `u32`, 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 that decode error into `NeverConsumable` for the whole note. A field the target branch never reads is therefore enough to hide a note the target could actually spend.
A reclaim height of exactly zero, conversely, makes the checker report a note as reclaimable when reclaim is permanently disabled. [`is_consumable_inner`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/mod.rs#L225-L286) resolves an absent reclaim height with `unwrap_or_default`, turning "disabled" into plain block zero, and takes the maximum of that and the timelock height. The reclaimer branch therefore reports the note as consumable once the timelock elapses, or immediately with no timelock, while the script rejects every reclaim attempt at every block with `ERR_P2IDE_RECLAIM_DISABLED`. This needs no unusual configuration, since the default reclaimer is the sender, distinct from the target, and is the common case.
In both directions, [`can_consume`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-tx/src/executor/notes_checker.rs#L216-L229) returns as soon as the static analysis yields any answer, so the batch path that executes the script and would otherwise expose the correct behavior is never reached for these notes. A wallet or indexer relying on this analysis hides a note its target could have consumed, or offers a reclaim that fails as soon as it is executed, wasting the work spent building it. The on-chain script itself is unaffected in every case; the divergence is confined to off-chain classification.
Consider handling the reclaim height as its own well-defined domain. For the target branch, consumability should not depend on decoding the reclaim height at all, since the script never reads it there. For the reclaimer branch, an absent or zero reclaim height should resolve to a `NeverConsumable` (or explicitly disabled) status rather than block zero, and a height exceeding the script's representable range should be handled explicitly rather than rejected outright, so that both branches of the off-chain analysis match what `assert_unlocked` and `reclaim_note` actually enforce on chain.
---
_Copied verbatim from finding [L-11](https://audits.openzeppelin.com/miden/miden-01-07-smart-contract-diff-audit-nfts/issues/off-chain-p2ide-consumability-analysis-diverges-from-note-script-reclaim-height-semantics-6acc13ba) (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`._
貢獻指南
評估
這個 Issue 還沒有評估資料。