Smart Contract Diff Audit L-16: Unvalidated `OnBlockSlot` Parameters Can Overflow the Consumability Window Computation
- Langage dominant
- Rust
- Étoiles
- 132
- Forks
- 167
- Merge moyen
- 1 j 23 h
- PR mergées (30 j)
- 110
Description
The `OnBlockSlot` variant of `NoteExecutionHint` describes a repeating window in which a note is expected to be consumable, encoded as three bytes named `round_len`, `slot_len`, and `slot_offset`. Neither the decoder in [`from_parts`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/execution_hint.rs#L100-L111), which checks only that the unused high payload byte is zero, nor the [`on_block_slot`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/execution_hint.rs#L81-L83) constructor places any bound on the three values or on their relationship to each other, so a payload in which all three bytes are `255` decodes without error. The values are fully controlled by the note author, since the network account target attachment stores the hint and [decodes it back](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/network_account_target.rs#L114-L120) without a bounds check.
The decoded bytes are consumed by [`can_be_consumed`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/execution_hint.rs#L126-L147) as shift amounts in `1 << round_len` and `1 << slot_len`, and then in unchecked `u32` multiplication and addition. A shift amount of 32 or greater panics when overflow checks are enabled, and is otherwise masked into the range 0 to 31, yielding a window unrelated to the encoded parameters. The multiplication of `slot_offset` by the slot length in blocks can exceed `u32::MAX` even when both shift amounts are in range, for instance with `round_len` of 1, `slot_len` of 25, and `slot_offset` of 255. That out-of-range values reach this arithmetic unhindered is illustrated by the crate's own [round-trip test](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-standards/src/note/execution_hint.rs#L231-L238), which encodes and decodes a `slot_len` of 33. The hint is advisory metadata that no kernel or note script enforces, and `can_be_consumed` has no caller outside its unit test in this repository, so the consequence falls on external consumers of the library: a consumer compiled with overflow checks aborts when evaluating a single malformed public note, while a consumer compiled without them silently computes an incorrect scheduling window.
Consider rejecting out-of-range `round_len`, `slot_len`, and `slot_offset` values at construction and decode time, in both `on_block_slot` and `from_parts`, so that an `OnBlockSlot` hint cannot exist with parameters its own window computation cannot represent. Alternatively, or in addition, consider making `can_be_consumed` total by widening the intermediate arithmetic to `u64` or by using the checked shift, multiplication, and addition operations and returning a defined result when the computation does not fit.
---
_Copied verbatim from finding [L-16](https://audits.openzeppelin.com/miden/miden-01-07-smart-contract-diff-audit-nfts/issues/unvalidated-onblockslot-parameters-can-overflow-the-consumability-window-computation-ed7294f8) (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`._
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.