diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f24bf27f..c783e3acf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Fixed `FungibleFaucet::receive_and_burn` treating a non-fungible asset issued by the same account as a fungible burn, which reduced `token_supply` without burning any fungible supply; the asset is now validated with the new `miden::standards::assets::fungible_asset::validate` procedure ([#3553](https://github.com/0xMiden/protocol/pull/3553)). - Fixed the authentication procedure not ending up at index 0 of an account's code when its MAST root was already exported by another component ([#3566](https://github.com/0xMiden/protocol/pull/3566)). - [BREAKING] Foreign procedure invocation now requires the provided procedure root to be part of the foreign account's code, so a caller can no longer execute arbitrary code under a foreign account's identity ([#3575](https://github.com/0xMiden/protocol/pull/3575)). +- Verified each input note's storage-item count and preimage against its authenticated storage commitment ([#3593](https://github.com/0xMiden/protocol/issues/3593)). - Fixed `PrivateOutputNote` construction and deserialization accepting attachment data that is not committed by the note header ([#3556](https://github.com/0xMiden/protocol/pull/3579)). - Fixed `input_note::remove_asset` leaving a dangling asset slot when a non-canonical fungible value produced an empty removal remainder ([#3591](https://github.com/0xMiden/protocol/pull/3606)). - Fixed `input_note::remove_asset` succeeding when asked to remove an empty or malformed asset ID instead of reporting the asset as not found ([#3592](https://github.com/0xMiden/protocol/pull/3607)). diff --git a/crates/miden-protocol/asm/kernels/transaction-core/src/prologue.masm b/crates/miden-protocol/asm/kernels/transaction-core/src/prologue.masm index 5b10892910..45da6d3ded 100644 --- a/crates/miden-protocol/asm/kernels/transaction-core/src/prologue.masm +++ b/crates/miden-protocol/asm/kernels/transaction-core/src/prologue.masm @@ -5,6 +5,7 @@ use miden::core::word use miden::tx_kernel_core::account use miden::protocol_utils::account_id +use miden::protocol_utils::mem as mem_utils use miden::tx_kernel_core::asset_vault use miden::tx_kernel_core::asset use {ASSET_SIZE} from miden::tx_kernel_core::asset @@ -20,6 +21,9 @@ use {BLOCK_DATA_SECTION_OFFSET, INPUT_VAULT_ROOT_PTR, KERNEL_PROCEDURES_PTR, PAR # Max U32 value, used for initializing the expiration block number const MAX_BLOCK_NUM = 0xFFFFFFFF +# Procedure locals in process_note_num_storage_items +const PROCESS_NOTE_STORAGE_PREIMAGE_LOC = 0 + # ERRORS # ================================================================================================= @@ -62,6 +66,12 @@ const ERR_PROLOGUE_NEW_ACCOUNT_NONCE_MUST_BE_ZERO = "new account must have a zer const ERR_PROLOGUE_NUMBER_OF_NOTE_STORAGE_ITEMS_EXCEEDED_LIMIT = "number of note storage items exceeded the maximum limit of 1024" +const ERR_PROLOGUE_NOTE_STORAGE_ITEMS_COUNT_MISMATCH = + "the specified number of note storage items does not match its commitment" + +const ERR_PROLOGUE_NOTE_STORAGE_DOES_NOT_MATCH_COMMITMENT = + "note storage does not match the commitment" + const ERR_PROLOGUE_NOTE_AUTHENTICATION_FAILED = "failed to authenticate note inclusion in block" const ERR_PROLOGUE_KERNEL_PROCEDURE_COMMITMENT_MISMATCH = @@ -631,11 +641,15 @@ proc compute_note_metadata_commitment # => [NOTE_METADATA_COMMITMENT] end -#! Checks that the number of note storage is within limit and stores it to memory. +#! Checks that the number of note storage items is within limit, stores it to memory, and verifies +#! the note's storage against its authenticated storage commitment. #! #! Inputs: #! Operand stack: [note_ptr] #! Advice stack: [num_storage_items] +#! Advice map: { +#! NOTE_STORAGE_COMMITMENT: [[STORAGE_ITEMS]] +#! } #! Outputs: #! Operand stack: [note_ptr] #! Advice stack: [] @@ -643,6 +657,17 @@ end #! Where: #! - note_ptr is the memory location for the input note. #! - num_storage_items is the note's number of storage items. +#! - NOTE_STORAGE_COMMITMENT is the note's storage commitment, already stored in note memory. +#! - STORAGE_ITEMS are the note's storage items. +#! +#! Locals: +#! - 0..1024: buffer to which the storage preimage is written (MAX_NOTE_STORAGE_ITEMS elements) +#! +#! Panics if: +#! - the number of storage items exceeds the maximum limit. +#! - the number of storage items from the advice map does not match the note's declared count. +#! - the note's storage does not match its storage commitment. +@locals(1024) proc process_note_num_storage_items # move the number of storage items from the advice stack to the operand stack adv_push @@ -654,7 +679,34 @@ proc process_note_num_storage_items # => [num_storage_items, note_ptr] # store the number of storage items into the memory - dup.1 exec.memory::set_input_note_num_storage_items + dup dup.2 exec.memory::set_input_note_num_storage_items + # => [num_storage_items, note_ptr] + + # load the note's authenticated storage commitment and a pointer to the preimage memory locals + dup.1 exec.memory::get_input_note_storage_commitment + # => [NOTE_STORAGE_COMMITMENT, num_storage_items, note_ptr] + + locaddr.PROCESS_NOTE_STORAGE_PREIMAGE_LOC movdn.5 + # => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, note_ptr] + + # load the storage preimage from the advice map onto the advice stack, prefixed with its + # element count and padded to the next multiple of 8 + adv.push_mapvaln.8 + # OS => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, note_ptr] + # AS => [advice_num_storage_items, [STORAGE_ITEMS]] + + # assert the item count from the advice map matches the note's declared count + adv_push dup.5 assert_eq.err=ERR_PROLOGUE_NOTE_STORAGE_ITEMS_COUNT_MISMATCH + # => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, note_ptr] + + # write the preimage to the memory locals and compute its commitment + movup.5 movup.5 + # => [num_storage_items, dest_ptr, NOTE_STORAGE_COMMITMENT, note_ptr] + + exec.mem_utils::pipe_elements_preimage_to_memory + # => [COMPUTED_COMMITMENT, NOTE_STORAGE_COMMITMENT, note_ptr] + + assert_eqw.err=ERR_PROLOGUE_NOTE_STORAGE_DOES_NOT_MATCH_COMMITMENT # => [note_ptr] end diff --git a/crates/miden-testing/src/kernel_tests/tx/test_prologue.rs b/crates/miden-testing/src/kernel_tests/tx/test_prologue.rs index f999eba7d1..30ea07885c 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_prologue.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_prologue.rs @@ -17,6 +17,7 @@ use miden_protocol::asset::{FungibleAsset, NonFungibleAsset}; use miden_protocol::block::account_tree::AccountIdKey; use miden_protocol::errors::tx_kernel::{ ERR_ACCOUNT_SEED_AND_COMMITMENT_DIGEST_MISMATCH, + ERR_PROLOGUE_NOTE_STORAGE_ITEMS_COUNT_MISMATCH, ERR_PROLOGUE_NUMBER_OF_NOTE_ASSETS_EXCEEDS_LIMIT, }; use miden_protocol::note::NoteId; @@ -202,6 +203,59 @@ async fn test_transaction_prologue_rejects_too_many_note_assets() -> anyhow::Res Ok(()) } +#[tokio::test] +async fn test_transaction_prologue_verifies_note_storage_against_commitment() -> anyhow::Result<()> +{ + // The number of storage items sits right after the 7 note-detail words in the note-data blob. + const NOTE_DATA_NUM_STORAGE_ITEMS_IDX: usize = 7 * WORD_SIZE; + + let assets = vec![NonFungibleAsset::mock(&0u32.to_le_bytes())]; + let input_note = create_public_p2any_note(ACCOUNT_ID_SENDER.try_into()?, assets); + let mut mock_tx = TestTransactionBuilder::with_existing_mock_account() + .input_note(input_note) + .build()?; + + let input_notes_commitment = mock_tx.input_notes().commitment(); + let (_, advice_inputs) = TransactionKernel::prepare_inputs(mock_tx.tx_inputs()); + let note_data = advice_inputs + .as_advice_inputs() + .map + .get(&input_notes_commitment) + .context("input-note advice should be present")? + .as_ref() + .to_vec(); + + let code = " + use miden::tx_kernel_core::prologue + + begin + exec.prologue::prepare_transaction + end + "; + + // The note has empty storage (count == 0): the prologue accepts it because the empty preimage + // hashes to the note's storage commitment. + assert_eq!(note_data[NOTE_DATA_NUM_STORAGE_ITEMS_IDX], ZERO); + mock_tx + .execute_code(code) + .await + .context("valid empty-storage note should be accepted")?; + + // Forge a non-zero storage-item count that the note's storage commitment does not attest to. + // The count stays within the limit, so it bypasses the bounds check and must be caught by + // the commitment verification instead. + let mut note_data = note_data; + note_data[NOTE_DATA_NUM_STORAGE_ITEMS_IDX] = Felt::from(1u32); + mock_tx.set_tx_args(TransactionArgs::new( + BTreeMap::from([(input_notes_commitment, note_data)]).into(), + )); + + let result = mock_tx.execute_code(code).await; + assert_execution_error!(result, ERR_PROLOGUE_NOTE_STORAGE_ITEMS_COUNT_MISMATCH); + + Ok(()) +} + fn global_input_memory_assertions(exec_output: &ExecutionOutput, inputs: &MockTransaction) { assert_eq!( exec_output.get_kernel_mem_word(BLOCK_COMMITMENT_PTR),