Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
- 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)).
- Storage slot types are now validated against the supported set at account creation, and the delta commitment rejects an unrecognized slot type instead of treating it as a map ([#3598](https://github.com/0xMiden/protocol/pull/3608)).
- Faucet asset-callback procedure roots are now verified against the faucet's account code before dispatch, so a misconfigured callback root can no longer make an asset nontransferable ([#3612](https://github.com/0xMiden/protocol/pull/3612)).
- [BREAKING] Enforced the limit of 1024 per asset delta op for added and removed account vault deltas inside and outside the tx kernel ([#3623](https://github.com/0xMiden/protocol/pull/3623)).

Expand Down
109 changes: 49 additions & 60 deletions crates/miden-protocol/asm/kernels/transaction-core/src/account.masm
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const ERR_ACCOUNT_PROC_NOT_AUTH_PROC =
const ERR_ACCOUNT_STORAGE_SLOTS_MUST_BE_SORTED_AND_UNIQUE =
"slot IDs must be unique and sorted in ascending order"

const ERR_ACCOUNT_STORAGE_SLOT_TYPE_IS_INVALID =
"account storage slot has an unsupported type"

const ERR_ACCOUNT_PROCEDURES_MUST_BE_SORTED_AND_UNIQUE =
"account procedures following the authentication procedure must be unique and sorted in ascending order"

Expand Down Expand Up @@ -945,61 +948,72 @@ pub proc validate_seed
# => []
end

#! Validates that slot IDs are sorted in ascending order and that slot IDs are unique.
#! Validates that slot IDs are sorted in ascending order, that slot IDs are unique, and that every
#! slot's type belongs to the set of supported types (value or map).
#!
#! This validates the storage of the native account, which is the only account whose storage is
#! validated this way: a foreign account's storage is instead bound to the commitment verified
#! against the account tree.
Comment thread
mmagician marked this conversation as resolved.
Outdated
#!
#! Inputs: []
#! Outputs: []
#!
#! Pancis if:
#! Panics if:
#! - each slot's ID is not strictly less than the next slot's ID.
#! - this ensures sorting and uniqueness among slot IDs.
#! - any slot has a type that is not a supported storage slot type.
pub proc validate_storage
exec.memory::get_num_storage_slots
exec.memory::get_native_num_storage_slots
# => [num_slots]

# compute the number of slot ID comparisons we need to make
# generally, we need num_storage_slots - 1 comparisons, e.g., if we have 3 slots, we need 2
# comparisons: 2 with 1 and 1 with 0.
# we subtract 1 if any slots exist and 0 otherwise, notably:
# maps 1 storage slot -> 0 comparisons, since 1 slot is always sorted and unique
# maps 0 storage slots -> 0 comparisons
dup u32gt.0
# => [has_slots, num_slots]

sub
# => [num_comparisons]

# loop if we need to compare slots
# iterate over all slots from the last one down to slot 0, validating each slot's type and
# comparing each slot's ID against the previous slot's ID
dup neq.0
# => [should_loop, num_comparisons]
# => [should_loop, num_slots]

while.true
# first iteration: number of comparisons = current slot index
# first iteration: remaining = num_slots, slot index = remaining - 1
# => [remaining]

sub.1
# => [curr_slot_idx]

dup exec.get_slot_id
# => [curr_slot_id_suffix, curr_slot_id_prefix, curr_slot_idx]
dup exec.get_native_storage_slot_type
# => [slot_type, curr_slot_idx]

# assert the slot type is value or map
dup eq.STORAGE_SLOT_TYPE_VALUE swap eq.STORAGE_SLOT_TYPE_MAP or
assert.err=ERR_ACCOUNT_STORAGE_SLOT_TYPE_IS_INVALID
# => [curr_slot_idx]

# we are guaranteed to not underflow because curr_slot_idx is at least 1 at the
# beginning of the loop
dup.2 sub.1
# => [prev_slot_idx, curr_slot_id_suffix, curr_slot_id_prefix, curr_slot_idx]
# compare the slot's ID against the previous slot's ID, unless this is slot 0
dup neq.0
# => [has_prev_slot, curr_slot_idx]

exec.get_slot_id
# => [prev_slot_id_suffix, prev_slot_id_prefix, curr_slot_id_suffix, curr_slot_id_prefix, curr_slot_idx]
if.true
dup exec.get_native_slot_id
# => [curr_slot_id_suffix, curr_slot_id_prefix, curr_slot_idx]

# this effectively checks that slots are sorted _and_ unique, since duplicate slot IDs are
# not less than each other
exec.is_slot_id_lt
# => [is_prev_lt_curr, curr_slot_idx]
# we are guaranteed to not underflow because curr_slot_idx is at least 1 in this branch
dup.2 sub.1
# => [prev_slot_idx, curr_slot_id_suffix, curr_slot_id_prefix, curr_slot_idx]

assert.err=ERR_ACCOUNT_STORAGE_SLOTS_MUST_BE_SORTED_AND_UNIQUE
# => [curr_slot_idx]
exec.get_native_slot_id
# => [prev_slot_id_suffix, prev_slot_id_prefix, curr_slot_id_suffix, curr_slot_id_prefix, curr_slot_idx]

sub.1 dup neq.0
# => [should_continue, prev_slot_idx]
# this effectively checks that slots are sorted _and_ unique, since duplicate slot IDs
# are not less than each other
exec.is_slot_id_lt
# => [is_prev_lt_curr, curr_slot_idx]

assert.err=ERR_ACCOUNT_STORAGE_SLOTS_MUST_BE_SORTED_AND_UNIQUE
# => [curr_slot_idx]
end

dup neq.0
# => [should_continue, curr_slot_idx]
end
# => [prev_slot_idx]
# => [curr_slot_idx]

drop
# => []
Expand Down Expand Up @@ -1569,31 +1583,6 @@ pub proc get_native_slot_id
# => [slot_id_suffix, slot_id_prefix]
end

#! Gets the slot ID of the storage slot at the provided index.
#!
#! WARNING: The index must be in bounds.
#!
#! Inputs: [index]
#! Outputs: [slot_id_suffix, slot_id_prefix]
#!
#! Where:
#! - index is the index of the slot.
#! - slot_id_{suffix, prefix} are the suffix and prefix felts of the slot identifier, which are
#! the first two felts of the hashed slot name.
pub proc get_slot_id
# convert the index into a memory offset
mul.ACCOUNT_STORAGE_SLOT_DATA_LENGTH
# => [offset]

exec.memory::get_account_active_storage_slots_section_ptr
add
# => [slot_ptr]

exec.get_slot_id_inner
# => [slot_id_suffix, slot_id_prefix]
end


#! Gets the slot ID of the storage slot pointed at by slot_ptr.
#!
#! WARNING: The slot_ptr must be valid.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use miden::tx_kernel_core::memory
use miden::core::crypto::hashes::poseidon2
use miden::core::word
use {ASSET_SIZE, COMPOSITION_NONE} from miden::tx_kernel_core::asset
use {STORAGE_SLOT_TYPE_VALUE} from miden::tx_kernel_core::constants
use {STORAGE_SLOT_TYPE_MAP, STORAGE_SLOT_TYPE_VALUE} from miden::tx_kernel_core::constants
use {ACCOUNT_UPDATE_ASSET_PTR} from miden::tx_kernel_core::memory

# ERRORS
Expand All @@ -23,6 +23,9 @@ const ERR_ACCOUNT_DELTA_TOO_MANY_ADDED_ASSETS =
const ERR_ACCOUNT_DELTA_TOO_MANY_REMOVED_ASSETS =
"number of removed assets in the transaction exceeds the maximum limit of 1024"

const ERR_ACCOUNT_STORAGE_SLOT_TYPE_IS_INVALID =
"account storage slot has an unsupported type"

# EVENTS
# =================================================================================================

Expand Down Expand Up @@ -278,12 +281,19 @@ proc commit_slot_patch
# => [storage_slot_type, slot_idx, RATE0, RATE1, CAPACITY]

# check if slot is of type value
push.STORAGE_SLOT_TYPE_VALUE eq
# => [is_value_slot_type, slot_idx, RATE0, RATE1, CAPACITY]
dup push.STORAGE_SLOT_TYPE_VALUE eq
# => [is_value_slot_type, storage_slot_type, slot_idx, RATE0, RATE1, CAPACITY]

if.true
drop
# => [slot_idx, RATE0, RATE1, CAPACITY]

exec.commit_value_slot_patch
else
# reject any type that is neither value nor map instead of defaulting to map
push.STORAGE_SLOT_TYPE_MAP eq assert.err=ERR_ACCOUNT_STORAGE_SLOT_TYPE_IS_INVALID
# => [slot_idx, RATE0, RATE1, CAPACITY]

exec.commit_map_slot_patch
end
# => [RATE0, RATE1, CAPACITY]
Expand Down
31 changes: 31 additions & 0 deletions crates/miden-testing/src/kernel_tests/tx/test_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use miden_protocol::errors::tx_kernel::{
ERR_ACCOUNT_NONCE_AT_MAX,
ERR_ACCOUNT_NONCE_CAN_ONLY_BE_INCREMENTED_ONCE,
ERR_ACCOUNT_PROCEDURES_MUST_BE_SORTED_AND_UNIQUE,
ERR_ACCOUNT_STORAGE_SLOT_TYPE_IS_INVALID,
ERR_ACCOUNT_UNKNOWN_STORAGE_SLOT_NAME,
};
use miden_protocol::field::PrimeField64;
Expand Down Expand Up @@ -594,6 +595,36 @@ async fn test_get_native_storage_slot_type() -> anyhow::Result<()> {
Ok(())
}

/// Tests that `validate_storage` rejects a storage slot whose type is outside the supported set
/// (value or map) instead of silently committing it as a map (audit finding L-11).
#[tokio::test]
async fn validate_storage_rejects_unsupported_slot_type() -> anyhow::Result<()> {
let mock_tx = TestTransactionBuilder::with_existing_mock_account().build().unwrap();

// Overwrite the type element (offset 1) of the first storage slot with an unsupported type,
// then run the new-account storage validation which must reject it.
let code = "
use miden::tx_kernel_core::account
use miden::tx_kernel_core::memory
use miden::tx_kernel_core::prologue

begin
exec.prologue::prepare_transaction

push.2
exec.memory::get_native_account_active_storage_slots_ptr add.1
mem_store

exec.account::validate_storage
end
";

let exec_output = mock_tx.execute_code(code).await;
assert_execution_error!(exec_output, ERR_ACCOUNT_STORAGE_SLOT_TYPE_IS_INVALID);

Ok(())
}

/// Tests that accessing an unknown slot fails with the expected error message.
///
/// This tests both accounts with empty storage and non-empty storage.
Expand Down
Loading