Skip to content

Commit b414de2

Browse files
partylikeits1983claudebobbinth
authored
fix: restrict indexed input note asset removal (#3451)
* fix: restrict indexed input note asset removal * refactor: share indexed input note removal guard * test: make malicious input note theft regression explicit * fix: require native account context for indexed input note asset removal The account-origin check resolves the caller against the *active* account, so the gate alone was bypassable: a malicious note script could route the indexed removal through an attacker-controlled foreign account via FPI, where that foreign account is active and vouches for its own procedures. The full #3445 drain still worked end to end against a standard wallet. Add exec.memory::assert_native_account to the guard, which closes that path while leaving the one legitimate by-index caller intact (the fee manager collects sponsorship note assets from the native account's auth procedure). Tests: a note-script-via-FPI regression test, and an explicit test that transaction scripts are rejected too - the latter pins a behavior change that was previously only implied by a deleted test. --------- Co-authored-by: Claude (Opus) <noreply@anthropic.com> Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>
1 parent a1214f9 commit b414de2

5 files changed

Lines changed: 421 additions & 157 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ Added a new `INPUT_NOTE_INDEX_LOOKUP_EVENT` that lets transaction hosts provide
5454

5555
### Fixes
5656

57+
- Restricted indexed input-note asset removal to the native account's context while preserving active-note self-removal. As a consequence, note scripts and transaction scripts can no longer remove input-note assets by index directly, and neither can foreign accounts invoked through FPI; indexed removal must go through a procedure of the native account ([#3445](https://github.com/0xMiden/protocol/issues/3445)).
5758
- Fixed `faucet::mint` and `faucet::burn` failing when the asset's witness in the input vault had not already been loaded, which happened when minting into a faucet whose vault held other assets, or when burning an asset the transaction had not otherwise accessed; both procedures now request the witness from the host before updating the input vault ([#3409](https://github.com/0xMiden/protocol/pull/3409)).
5859
- Enforced the canonical encoding of `Authority` role map values on read: `Authority::try_from_storage` now rejects a procedure-role value word whose reserved felts (`value[1..=3]`) are non-zero, matching the value-slot check and completing the fix started in [#3209](https://github.com/0xMiden/protocol/pull/3209) ([#3415](https://github.com/0xMiden/protocol/pull/3415)).
5960

‎crates/miden-protocol/asm/kernels/transaction/lib/api.masm‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,9 @@ end
11191119
#!
11201120
#! Panics if:
11211121
#! - the note index is greater or equal to the total number of input notes.
1122-
#! - is_active_note is 1 and no input note is being processed (attempted to access note assets
1122+
#! - is_active_note is false and the active account is not the native account.
1123+
#! - is_active_note is false and the invocation does not originate from the account context.
1124+
#! - is_active_note is true and no input note is being processed (attempted to access note assets
11231125
#! from incorrect context).
11241126
#! - the asset's composition is Custom (not yet supported).
11251127
#! - the asset is not present in the note.
@@ -1132,6 +1134,9 @@ pub proc input_note_remove_asset
11321134
movup.9 swap
11331135
# => [is_active_note, note_index, ASSET_ID, ASSET_VALUE, pad(6)]
11341136

1137+
exec.authenticate_indexed_input_note_asset_removal
1138+
# => [is_active_note, note_index, ASSET_ID, ASSET_VALUE, pad(6)]
1139+
11351140
# get the input note pointer depending on whether the requested note is the active one or if it
11361141
# was requested by index.
11371142
exec.get_requested_note_ptr
@@ -1175,11 +1180,16 @@ end
11751180
#!
11761181
#! Panics if:
11771182
#! - the note index is greater or equal to the total number of input notes.
1178-
#! - is_active_note is 1 and no input note is being processed (attempted to access note assets
1183+
#! - is_active_note is false and the active account is not the native account.
1184+
#! - is_active_note is false and the invocation does not originate from the account context.
1185+
#! - is_active_note is true and no input note is being processed (attempted to access note assets
11791186
#! from incorrect context).
11801187
#!
11811188
#! Invocation: dynexec
11821189
pub proc input_note_remove_all_assets
1190+
exec.authenticate_indexed_input_note_asset_removal
1191+
# => [is_active_note, note_index, pad(14)]
1192+
11831193
# get the input note pointer depending on whether the requested note is the active one or if it
11841194
# was requested by index.
11851195
exec.get_requested_note_ptr
@@ -2074,6 +2084,30 @@ end
20742084
# HELPER PROCEDURES
20752085
# =================================================================================================
20762086

2087+
#! Authenticates indexed input-note asset removal while allowing active-note self-removal.
2088+
#!
2089+
#! The native account check is required in addition to the account-origin check: the latter
2090+
#! resolves the caller against the active account, so without it a foreign account reached through
2091+
#! FPI could authorize the removal with its own procedures.
2092+
#!
2093+
#! Inputs: [is_active_note]
2094+
#! Outputs: [is_active_note]
2095+
#!
2096+
#! Where:
2097+
#! - is_active_note is the boolean flag indicating whether the assets are removed from the active
2098+
#! note or from the note with the specified index.
2099+
#!
2100+
#! Panics if:
2101+
#! - is_active_note is false and the active account is not the native account.
2102+
#! - is_active_note is false and the invocation does not originate from the account context.
2103+
proc authenticate_indexed_input_note_asset_removal
2104+
dup eq.0
2105+
if.true
2106+
exec.memory::assert_native_account
2107+
exec.authenticate_account_origin
2108+
end
2109+
end
2110+
20772111
#! Returns the memory pointer to the input note, depending on whether the requested note is current
20782112
#! or it was requested by index.
20792113
#!

‎crates/miden-protocol/asm/protocol/src/input_note.masm‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ end
265265
#! the EMPTY_WORD if the entire asset was removed.
266266
#!
267267
#! Panics if:
268+
#! - the invocation does not originate from the native account's account context.
268269
#! - the note index is greater or equal to the total number of input notes.
269270
#! - the asset's composition is Custom (not yet supported).
270271
#! - the asset is not present in the note.
@@ -348,6 +349,7 @@ end
348349
#! - num_assets is the number of assets removed by this procedure.
349350
#!
350351
#! Panics if:
352+
#! - the invocation does not originate from the native account's account context.
351353
#! - the note index is greater or equal to the total number of input notes.
352354
#!
353355
#! Invocation: exec

0 commit comments

Comments
 (0)