Smart Contract Diff Audit L-13: Note Attachment Limits Are Enforced Only After Full Collections Are Parsed and Hashed
- Lenguaje dominante
- Rust
- Estrellas
- 132
- Forks
- 167
- Merge medio
- 1 d 23 h
- PR fusionados (30 d)
- 110
Descripción
[`NoteAttachments::read_from`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/attachment/mod.rs#L625-L631) reads an attachment count from a single untrusted byte and then deserializes that many `NoteAttachment` values into a vector before any protocol limit is applied. Both limits, `MAX_COUNT` (4) and `MAX_NUM_WORDS` (512), are checked by [`NoteAttachments::new`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/attachment/mod.rs#L502-L517), which runs only once the whole vector has been materialized. Each attachment parsed along the way has already had its words copied into an element buffer and hashed by [`NoteAttachmentContent::new`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/attachment/mod.rs#L164-L177).
The per-attachment limit supplies no earlier bound either: [`MAX_NUM_WORDS`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/attachment/mod.rs#L54) is 256 words, while the wire format encodes the word count as a byte holding the count minus one, so the value reconstructed in [`NoteAttachmentContent::read_from`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/attachment/mod.rs#L224-L232) can never exceed 256 and that check can never reject a value read from the wire. Nothing therefore rejects an attachment collection until the final aggregate check, so a collection far larger than the protocol permits is parsed and hashed in full before it is discarded. [`Note::read_from`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/mod.rs#L287-L294) reaches this path for every note and every transaction that is decoded.
The discarded work is bounded by the size of the input, because each declared word must be backed by thirty-two bytes actually present in the reader and the collected vector never preallocates the declared count. A service that limits the size of accepted messages therefore also limits this path. That bound is left entirely to callers: the crate re-exports [`BudgetedReader`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/utils/mod.rs#L14) and the associated `read_from_bytes_with_budget` entry point, but no deserialization performed inside the crate runs under a budget.
Consider validating the declared count against `NoteAttachments::MAX_COUNT` immediately after it is read, and accumulating the total word count while attachments are parsed, so that an oversized collection is rejected before it is materialized and hashed. The same fail-late idiom appears in the deserialization of [`NoteStorage`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/storage.rs#L132-L136), [`NoteAssets`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/note/assets.rs#L175-L179), [`InputNotes`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/transaction/inputs/notes.rs#L205-L209), and [`OutputNoteCollection`](https://github.com/0xMiden/protocol/blob/8411bf093bde25285708faac152b6d7269009617/crates/miden-protocol/src/transaction/outputs/notes.rs#L163-L167), so any fix is best applied consistently across them.
---
_Copied verbatim from finding [L-13](https://audits.openzeppelin.com/miden/miden-01-07-smart-contract-diff-audit-nfts/issues/note-attachment-limits-are-enforced-only-after-full-collections-are-parsed-and-hashed-38f5fa5a) (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
Evaluación
Este issue todavía no se ha evaluado.