Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
107 changes: 47 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,70 @@ 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).
#!
#! Validation should only ever happen on the storage of the native account.
#!
#! 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 +1581,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