0xMiden / 0xMiden/protocol

Smart Contract Diff Audit L-11: Off-Chain P2IDE Consumability Analysis Diverges From Note Script Reclaim-Height Semantics

オープン
#3,675 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Rust
スター
132
フォーク
167
平均マージ
1日 23時間
マージ済み PR(30日)
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 はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。