Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion bin/bench-transaction/src/context_setups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ pub async fn tx_consume_b2agg_note(pre_populate_leaves: Option<u32>) -> Result<T
mock_chain.prove_next_block()?;

// TX1: BUILD B2AGG NOTE TRANSACTION CONTEXT (ready to execute)
let burn_note_script = StandardNote::BURN.script();
let burn_note_script = StandardNote::BURN.script()?;
let foreign_account_inputs = mock_chain.get_foreign_account_inputs(faucet.id())?;
let b2agg_tx_context = mock_chain
.build_tx_context(bridge_account.id(), &[b2agg_note.id()], &[])?
Expand Down
21 changes: 17 additions & 4 deletions crates/miden-standards/asm/standards/notes/pswap.masm
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const EXEC_AMT_REQUESTED_NOTE_FILL = 9
# write the word at PARENT_ATTACHMENT_PTR, read back only the depth at DEPTH_OFFSET.
const PARENT_ATTACHMENT_PTR = 0
const PARENT_ATTACHMENT_DEPTH_OFFSET = 2
const PSWAP_ATTACHMENT_NUM_WORDS = 1

# ERRORS
# =================================================================================================
Expand All @@ -106,6 +107,8 @@ const ERR_PSWAP_FILL_EXCEEDS_REQUESTED="PSWAP fill amount exceeds requested amou
const ERR_PSWAP_FILL_SUM_OVERFLOW="PSWAP account_fill + note_fill overflows u64"
const ERR_PSWAP_NOT_VALID_ASSET_AMOUNT="PSWAP computed amount exceeds max fungible asset amount"
const ERR_PSWAP_PAYOUT_OVERFLOW="PSWAP payout quotient does not fit in u64"
const ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS="PSWAP attachment must encode exactly one word"
const ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32="PSWAP attachment depth must fit in u32"

# U64 VALIDATION
# =================================================================================================
Expand Down Expand Up @@ -493,9 +496,13 @@ proc get_order_id
end

#! Returns current_depth = parent_depth + 1, where parent_depth is the depth carried in the
#! consumed PSWAP note's PswapAttachment word at offset PARENT_ATTACHMENT_DEPTH_OFFSET if such an
#! attachment exists, or 0 if not (i.e., the parent is the original PSWAP, with scheme none()
#! or NetworkAccountTarget).
#! consumed PSWAP note's PswapAttachment word at offset PARENT_ATTACHMENT_DEPTH_OFFSET if such
#! an attachment exists, or 0 if not.
#!
#! The initial (depth-0) PSWAP carries no PSWAP_ATTACHMENT_SCHEME attachment; only payback P2IDs
#! and remainder PSWAPs do. So `is_found = false` here means we are consuming the original PSWAP
#! (or one carrying only an unrelated scheme like NetworkAccountTarget) and the next round is
#! depth 1.
#!
#! Inputs: []
#! Outputs: [current_depth]
Expand All @@ -514,11 +521,17 @@ proc get_current_depth
exec.active_note::write_attachment_to_memory
# => [num_words]

drop
eq.PSWAP_ATTACHMENT_NUM_WORDS
assert.err=ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS
Comment on lines 521 to +525

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The attachment-length guard runs after the unsafe write.

write_attachment_to_memory has already copied the payload into a 4-local buffer before num_words == 1 is asserted, so a forged multi-word PSWAP attachment can still clobber adjacent local memory and only then fail. If there is no length-only probe, this needs a scratch region sized for the maximum attachment length or another pre-write bound check; the current check does not provide the protection described in the comment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/miden-standards/asm/standards/notes/pswap.masm` around lines 522 -
528, The current guard (eq.PSWAP_ATTACHMENT_NUM_WORDS /
assert.err=ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS) runs after
exec.active_note::write_attachment_to_memory has already copied into the 4-word
`@locals`(4) buffer, so a forged multi-word attachment can clobber locals before
failing; fix by performing the attachment-length check before any write (i.e.,
check num_words == 1 immediately in exec.active_note::write_attachment_to_memory
or its caller) or else allocate/use a scratch region sized for the maximum
allowed attachment length and validate bounds prior to writing; reference
exec.active_note::write_attachment_to_memory, `@locals`(4),
eq.PSWAP_ATTACHMENT_NUM_WORDS and ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS when
making the change.

# => []

loc_load.PARENT_ATTACHMENT_DEPTH_OFFSET
# => [parent_depth]
dup u32split drop
# => [hi, parent_depth]

assertz.err=ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line after # =>

# => [parent_depth]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this long comment
And see if their direct way to check if value is u32 or not

else
drop push.0
# => [0]
Expand Down
54 changes: 35 additions & 19 deletions crates/miden-standards/src/note/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ mod p2ide;
pub use p2ide::{P2ideNote, P2ideNoteStorage};

mod pswap;
pub use pswap::{PswapNote, PswapNoteAttachment, PswapNoteStorage};
pub use pswap::{OrderId, PswapNote, PswapNoteAttachment, PswapNoteStorage};

mod swap;
pub use swap::{SwapNote, SwapNoteStorage};
Expand Down Expand Up @@ -55,33 +55,41 @@ impl StandardNote {

/// Returns a [`StandardNote`] instance based on the provided [`NoteScript`]. Returns `None`
/// if the provided script does not match any standard note script.
pub fn from_script(script: &NoteScript) -> Option<Self> {
///
/// # Errors
///
/// Propagates the script-load error from [`PswapNote::script_root`] (build-time invariant).
pub fn from_script(script: &NoteScript) -> Result<Option<Self>, NoteError> {
Self::from_script_root(script.root())
}

/// Returns a [`StandardNote`] instance based on the provided script root. Returns `None` if
/// the provided root does not match any standard note script.
pub fn from_script_root(root: NoteScriptRoot) -> Option<Self> {
///
/// # Errors
///
/// Propagates the script-load error from [`PswapNote::script_root`] (build-time invariant).
pub fn from_script_root(root: NoteScriptRoot) -> Result<Option<Self>, NoteError> {
if root == P2idNote::script_root() {
return Some(Self::P2ID);
return Ok(Some(Self::P2ID));
}
if root == P2ideNote::script_root() {
return Some(Self::P2IDE);
return Ok(Some(Self::P2IDE));
}
if root == SwapNote::script_root() {
return Some(Self::SWAP);
return Ok(Some(Self::SWAP));
}
if root == PswapNote::script_root() {
return Some(Self::PSWAP);
if root == PswapNote::script_root()? {
return Ok(Some(Self::PSWAP));
}
if root == MintNote::script_root() {
return Some(Self::MINT);
return Ok(Some(Self::MINT));
}
if root == BurnNote::script_root() {
return Some(Self::BURN);
return Ok(Some(Self::BURN));
}

None
Ok(None)
}

// PUBLIC ACCESSORS
Expand Down Expand Up @@ -112,27 +120,35 @@ impl StandardNote {
}

/// Returns the note script of the current [StandardNote] instance.
pub fn script(&self) -> NoteScript {
match self {
///
/// # Errors
///
/// Propagates the script-load error from [`PswapNote::script`] (build-time invariant).
pub fn script(&self) -> Result<NoteScript, NoteError> {
Ok(match self {
Self::P2ID => P2idNote::script(),
Self::P2IDE => P2ideNote::script(),
Self::SWAP => SwapNote::script(),
Self::PSWAP => PswapNote::script(),
Self::PSWAP => PswapNote::script()?,
Self::MINT => MintNote::script(),
Self::BURN => BurnNote::script(),
}
})
}

/// Returns the script root of the current [StandardNote] instance.
pub fn script_root(&self) -> NoteScriptRoot {
match self {
///
/// # Errors
///
/// Propagates the script-load error from [`PswapNote::script_root`] (build-time invariant).
pub fn script_root(&self) -> Result<NoteScriptRoot, NoteError> {
Ok(match self {
Self::P2ID => P2idNote::script_root(),
Self::P2IDE => P2ideNote::script_root(),
Self::SWAP => SwapNote::script_root(),
Self::PSWAP => PswapNote::script_root(),
Self::PSWAP => PswapNote::script_root()?,
Self::MINT => MintNote::script_root(),
Self::BURN => BurnNote::script_root(),
}
})
}

/// Performs the inputs check of the provided standard note against the target account and the
Expand Down
Loading
Loading