0xMiden / 0xMiden/protocol

Smart Contract Diff Audit L-26: P2IDE Notes With Out-Of-Range Reclaim Heights Are Spendable On Chain But Reported As Never Consumable

オープン
#3,685 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Rust
スター
132
フォーク
167
平均マージ
1日 23時間
マージ済み PR(30日)
110

説明

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`._

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

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

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