0xMiden / 0xMiden/protocol

Smart Contract Diff Audit L-16: Unvalidated `OnBlockSlot` Parameters Can Overflow the Consumability Window Computation

Abierto
#3,679 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Rust
Estrellas
132
Forks
167
Merge medio
1 d 23 h
PR fusionados (30 d)
110

Descripción

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

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.