S&P Global Completes Acquisition of OpenZeppelin Read the announcement
Talk to an Expert

Miden Transaction Kernel Audit

  • OpenZeppelin Security

Findings
Total issues: 18 (9 resolved, 3 partially resolved)
Critical: 1 (1 resolved) · High: 2 (2 resolved) · Medium: 3 (3 resolved) · Low: 7 (3 resolved, 3 partially resolved)

Notes & Additional Information
5 notes raised (0 resolved)

Scope

OpenZeppelin performed an audit of the 0xMiden/miden-base repository at commit 37dd717.

In scope were the following files:

crates/miden-protocol/asm/kernels/transaction
    ├── lib
    │   ├── account.masm
    │   ├── account_delta.masm
    │   ├── asset.masm
    │   ├── asset_vault.masm
    │   ├── constants.masm
    │   ├── epilogue.masm
    │   ├── faucet.masm
    │   ├── input_note.masm
    │   ├── link_map.masm
    │   ├── memory.masm
    │   ├── note.masm
    │   ├── output_note.masm
    │   ├── prologue.masm
    │   └── tx.masm
    ├── api.masm
    ├── main.masm
    └── tx_script_main.masm

A previous security assessment was performed on the same scope. The findings from that engagement are contained in a separate report and have not been reproduced in this report.

System Overview

The Miden transaction kernel is the core program that executes and validates state transitions for accounts on the Miden rollup. Written in Miden assembly (MASM) and executed inside the Miden VM, it orchestrates the full lifecycle of a transaction, from ingesting inputs through producing a provably correct output state. The kernel operates as a trusted execution environment whose procedures are callable by untrusted account, note, and transaction script code through a controlled API, enforcing all protocol-level invariants such as asset preservation, access control, and storage integrity.

Transaction Lifecycle

A transaction executes against a single account, processes a set of input notes, and produces a new account state along with a set of output notes. The kernel orchestrates this through four sequential phases.

Prologue

The prologue ingests the transaction's public inputs (a reference block commitment, the account's initial commitment, an input notes commitment, the account ID, and the reference block number) and loads all private data from the advice provider. It authenticates block data, chain data, the target account's state, and input notes against Merkle commitments before storing them in memory. By the end of this phase, the VM's memory holds the complete, authenticated starting state of the transaction.

Note Processing

The kernel iterates over all input notes and executes each note's script via dyncall. Notes are the fundamental mechanism for transferring assets and triggering logic between accounts. Each note carries a serial number, a script root, assets, input data, and metadata. A recipient commitment is derived from the serial number, script root, and inputs commitment. Note scripts run in a new context and may invoke kernel API procedures to move assets or create new output notes.

Transaction Script

If present, an optional transaction script executes via dyncall after all notes have been processed. It is typically used for account-level operations such as asset transfers not tied to any specific note.

Epilogue

The epilogue finalizes the transaction. It runs the account's authentication procedure (which must increment the nonce if the account state changes), then enforces asset preservation by comparing the input vault root against the output vault root. The kernel computes the transaction fee from the cycle count, removes it from the account vault, and produces the final output stack containing the output notes commitment, the account update commitment, the fee asset, and the expiration block number.

Transaction Inputs and Authentication

The transaction consumes a small set of public inputs committed to in the proof and a larger body of private inputs from the advice provider. Public inputs consist of three words and three field elements: the reference block commitment, the initial account commitment (empty for new accounts), the input notes commitment, the account ID prefix, the account ID suffix, and the reference block number.

The advice provider supplies the complete data behind each commitment: full block header fields, account state, per-note data (serial number, script root, assets, metadata, recipient), and Merkle authentication paths. The prologue reconstructs each public commitment from this private data and asserts equality. Block fields are hashed against the block commitment, account state words against the initial account commitment, and (nullifier, note commitment) pairs against the input notes commitment. Notes are further verified against their creation block's note tree. The advice provider is never trusted, as every private value is verified against a public root before it influences execution.

Transaction Outputs

The kernel produces four outputs on the stack:

  • OUTPUT_NOTES_COMMITMENT: A sequential hash over all output notes, computed from each note's ID and metadata commitment.
  • ACCOUNT_UPDATE_COMMITMENT: The hash of the final account commitment and the account delta commitment, encoding all changes applied during the transaction.
  • FEE_ASSET: The transaction fee denominated in the network's native asset, removed from the account vault after the asset preservation check.
  • tx_expiration_block_num: The block number after which the transaction is considered expired.

Kernel API

The kernel API is the sole interface through which untrusted code (note scripts, transaction scripts, account procedures) interacts with privileged kernel state. Procedures are invoked via syscall.

The API is organized into the following categories:

  • Account State: Read-only access to account ID, nonce, code commitment, and computed commitments. Available to any code in the account context.
  • Account Storage: Reading and writing key-value storage slots and map entries. Write operations are restricted to the native account and require caller authentication.
  • Account Vault: Adding and removing fungible and non-fungible assets, querying balances, and checking asset presence. Mutations are restricted to the native account.
  • Account Procedures: Introspecting account code (procedure count, roots, existence checks, call tracking).
  • Nonce Management: account_incr_nonce is the most restricted operation, requiring the caller to be the native account's authentication procedure. The nonce can only be incremented once per transaction.
  • Faucet Operations: Minting and burning assets, querying issuance state. Restricted to the native account, with caller authentication and faucet-type validation.
  • Input Note Access: Reading note data (assets, recipient, metadata, serial number, inputs, script root) by active-note or by index.
  • Output Note Management: Creating notes, adding assets, setting attachments, and reading back note data. Creation and mutation restricted to the native account.
  • Transaction Data: Access to block-level metadata, note commitments, note counts, and expiration delta management. All operations are read-only except tx_update_expiration_block_delta, which sets the transaction expiration block.
  • Foreign Account Context: Opening and closing a foreign account context to read another account's public state or execute its code.

Security Model and Trust Assumptions

The transaction kernel enforces protocol invariants within the Miden VM's provable execution model. All kernel procedures execute in a privileged context that is inaccessible to user-supplied code except through the controlled API surface. The security of the system rests on the following assumptions.

  • The compilation of MASM into Miden VM instructions is correct.
  • The underlying Miden VM is properly constrained.
  • The Miden VM correctly executes the kernel code and produces a valid zero-knowledge proof of the execution trace. All arithmetic, memory operations, and cryptographic primitives (hashing, Merkle tree operations) behave correctly.
  • The batch and block kernels are expected to validate that the input notes are unique and to implement deferred validation of notes whose authentication was delayed by the transaction kernel.
  • Block data, chain data, account state, and input notes are authenticated against commitments provided as global inputs. The kernel assumes that these root commitments correspond to the canonical chain state.
  • The VM's context separation mechanism prevents unauthorized cross-context calls. It is assumed that user programs execute in separate contexts and may only interact with the root context through controlled syscalls defined in api.masm.
  • Account owners are responsible for deploying correct code.
  • The user employs a secure signature-based auth module and verifies all input data, output notes, and the account delta before signing.

Critical Severity

Empty Link Map Bypasses Entry Pointer Validation

The assert_entry_ptr_is_valid procedure in link_map.masm enforces safety checks on host-provided entry pointers, ensuring that they fall within the valid memory region, are properly aligned, and belong to the correct map. These checks are essential for preventing unauthorized memory access within the kernel.

However, when the link map is empty, the procedure unconditionally bypasses all three validation checks by OR-ing the result of each assertion with the is_empty_map flag. This logic flaw allows a malicious prover to supply an arbitrary entry_ptr when operating on an empty map. Since the link_map::set and link_map::get procedures execute logic based on the host-provided operation code regardless of the map's state, an attacker can exploit this bypass to read from or write to any kernel memory address.

The attack proceeds as follows:

  1. The attacker selects a map_ptr that points to an empty map (zero value).
  2. The attacker calls link_map::set with a target key/value, providing a host advice operation of INSERT_OPERATION_UPDATE (0) and an entry_ptr pointing to a sensitive kernel memory address.
  3. assert_entry_ptr_is_valid is called, but all checks pass because is_empty_map is true.
  4. Execution proceeds to update_entry because the operation is UPDATE, which calls set_value to write the attacker's value to the arbitrary entry_ptr address.

A similar flow applies to link_map::get with GET_OPERATION_FOUND, allowing arbitrary memory reads while returning fabricated values. Additionally, this bypass enables the creation of "orphaned" entries, valid entries inserted via INSERT_AFTER_ENTRY on an empty map that are never linked to the map head. These entries remain invisible to iteration-based routines like commitment computation, allowing state mutations that escape cryptographic verification.

Consider restricting valid operations on empty maps to only accept INSERT_OPERATION_AT_HEAD for sets and GET_OPERATION_ABSENT_AT_HEAD for gets, rejecting all other operations to prevent misuse. In addition, as an optimization, the assert_entry_ptr_is_valid call could be skipped when the operation is InsertAtHead or AbsentAtHead, since in these cases the potentially invalid entry pointer is discarded.

Update: Resolved in pull request #2556.

High Severity

Active Storage Slot Memory Region May Be Overwritten

The transaction kernel maintains a snapshot of the native account’s initial storage slots in order to compute a storage delta later in the transaction. In the in-memory native account data section, the initial storage slots are copied to ACCT_INITIAL_STORAGE_SLOTS_SECTION_OFFSET = 1320, and the active slots to ACCT_ACTIVE_STORAGE_SLOTS_SECTION_OFFSET = 2340.

However, the reserved gap between these sections is only 2340 - 1320 = 1020 field elements. Since each storage slot occupies ACCOUNT_STORAGE_SLOT_DATA_LENGTH = 8 field elements, the snapshot can hold at most ⌊1020/8⌋ = 127 full slots without overlap, which is insufficient to cover the required MAX_NUM_STORAGE_SLOTS = 255. As a result, copying the active storage slots into the initial storage slot region may overwrite part of the active storage region.

Consider updating the memory layout to ensure that at least 2040 = 255 * 8 memory addresses are allocated for each of the initial and active storage slot sections. Note that this issue was recently introduced in PR #2190.

Update: Resolved in pull request #2557.

Foreign Context Can Load and Validate a Different Account Than the One Requested

The load_foreign_account procedure is responsible for loading a foreign account's data from the advice provider into memory, using the requested account ID as a key for the advice map lookup.

However, load_foreign_account does not verify that the account ID returned by the advice provider matches the requested account ID. It writes the ID provided by the advice map into the active account memory slot via memory::set_account_id_and_nonce without comparison. Since the advice map is prover-controlled, a malicious prover can provide data for a different account. The subsequent validation in validate_active_foreign_account fails to catch this because it retrieves the account ID from memory—which now contains the substituted ID—to verify the account's existence in the account database. As long as the substituted account is valid, the check passes, allowing the kernel to execute a foreign context with an unintended account.

Consider modifying load_foreign_account to assert that the account ID extracted from the advice-provided data matches the requested account ID before writing it to memory.

Update: Resolved in pull request #2635.

Medium Severity

Invalid Note Indexes Are Accepted in Output Note Syscalls

In output_note.masm, the add_asset and set_attachment procedures accept a note_idx input via the operand stack. To ensure this index points to an existent output note, they perform the following check: exec.memory::get_num_output_notes lte.

However, the above check allows passing an index equal to the total amount of output notes, which results in an off-by-one error given the index is zero-based. Therefore, the attachment or assets may be added to a non-existent note (which may be created later in the transaction) by mistake, which could result in the loss of the assets and incorrect note behavior.

Consider updating the check to use lt instead of lte, and adjusting the error message accordingly. Alternatively, the assert_note_index_in_bounds procedure could be used to perform the check, as done in other output note syscalls.

Update: Resolved in pull request #2792. The team stated:

“Fixed by consistently using assert_note_index_in_bounds to implement the correct check.”

Non-Fungible Faucet Issuance Tracking Can Be Bypassed via account_set_map_item

Non-fungible faucets maintain a storage map in the reserved FAUCET_SYSDATA_SLOT that records every minted NFT. When mint_non_fungible_asset is invoked, it inserts the new asset into this map and asserts that the previous value at the corresponding key was empty, preventing duplicate issuance. To protect this invariant from direct slot overwrites, the account_set_item syscall checks whether the caller is a faucet and the target slot is the reserved faucet data slot, reverting if both conditions hold.

However, the account_set_map_item syscall performs no equivalent guard. It verifies only that the caller is the native account and that the invocation originates from an account procedure, then delegates unconditionally to the underlying storage write. Since a non-fungible faucet's sysdata slot is itself a map-type slot, with a custom procedure, a non-fungible faucet can call account_set_map_item targeting this slot to arbitrarily modify or erase individual entries in the issuance map.

Consider mirroring the guard from account_set_item in account_set_map_item. Before allowing the write, check whether the caller is a faucet and the target slot is the reserved faucet data slot, and revert if so.

Update: Resolved. The team stated:

“The sysdata slot was removed in newer versions (https://github.com/0xMiden/protocol/pull/2335) and so the issue no longer applies, hence we did not make a fix against the audited commit.”

Potential Desynchronization Between Asset Vault and Account Delta

In Miden, asset vaults are represented as Sparse Merkle Trees (SMTs), which store key-value pairs with 4-element keys and 4-element values. For non-fungible assets (NFTs), the stored value is ASSET = [hash0, hash1, hash2, faucet_id_prefix] (little-endian) and the corresponding key is ASSET_KEY = [faucet_id_prefix, hash1, hash2, hash0'] (little-endian), where hash0' equals hash0 with the "fungible bit" set to 0. The hash* values are derived from hashing the NFT's data.

Similarly, the native account's delta uses a link_map to track changes in the account's vault. However, for NFTs, the link_map uses the full ASSET as the key rather than computing and using the corresponding ASSET_KEY, as done by the asset vault. This inconsistency can result in misleading account deltas.

For example, two distinct NFT ASSET values could be crafted such that they differ only in the fungible bit of hash0, meaning they would map to the same ASSET_KEY and thus represent the same NFT from the vault's perspective. However, the account delta would treat them as separate assets. As a result, adding asset A and removing asset B, which effectively cancel out at the vault level, would appear in the delta as the acquisition of A and the loss of B, as if they were different NFTs.

Consider computing and using the same ASSET_KEY in the account delta logic, and clearly documenting that only ASSET_KEY should be relied upon to uniquely identify an NFT. Additionally, in remove_non_fungible_asset, consider asserting not only that OLD_VAL is non-empty, but also that it matches the input ASSET.

Update: Resolved. The team stated:

“Due to a recent refactor of assets from 1 to 2 words, this issue no longer applies and is now effectively fixed in account_delta::add_non_fungible_asset (which takes KEY and VALUE as input). The non-fungible asset key now contains the faucet ID while the value does not, so it is necessary to use the key in any case (https://github.com/0xMiden/protocol/pull/2437).”

Low Severity

Misleading Stack State in Kernel Procedures

Kernel procedure calls are dispatched by the exec_kernel_proc procedure, which is expected to return exactly 16 elements on the stack, as it is invoked via the syscall instruction.

However, several kernel procedures appear to return with fewer than 16 elements on the stack. For example, at the end of account_set_item, the account::set_item procedure is executed with the stack state [slot_id_prefix, slot_id_suffix, VALUE, pad(10)]. The set_item procedure consumes all non-padding elements and returns OLD_VALUE. After execution, the documented stack state is [OLD_VALUE, pad(12)], meaning the number of padding elements increases from 10 to 12 without an obvious explanation.

To improve clarity, consider explicitly documenting how padding is handled in these cases and clarifying why the stack returns to the expected size.

Update: Resolved. The team stated:

“These stack states are basically a consequence of the Miden VM’s “auto padding” to at least 16 elements. Since this issue is pervasive, we may want to document occurrences of this explicitly as suggested, but we deem it low priority for now.
The important part is that user-facing docs are not affected by this. It is internal to call-ed procedures.”

Notes Cannot Have the Maximum Number of Assets Attached

The set_output_note_num_assets procedure in memory.masm stores the number of assets of a given note in memory. Before storing the value, however, the function performs the following check: push.MAX_ASSETS_PER_NOTE lt. This enforces the number of assets to be strictly less than the maximum, when the maximum (256) should be a valid value as well. For instance, within prologue.masm, the process_note_assets procedure correctly implements the same check using push.MAX_ASSETS_PER_NOTE lte.

Consider updating the check in set_output_note_num_assets to use the lte instruction instead of lt. Additionally, the ERR_NOTE_NUM_OF_ASSETS_EXCEED_LIMIT error should be updated accordingly.

Update: Resolved in pull request #2741.

Transaction Script Program Does Not Load the Corresponding Arguments

The transaction script program implemented in tx_script_main.masm runs the prologue to prepare the transaction’s root context and then executes the provided transaction script.

However, unlike the main transaction kernel program, the transaction script arguments are not loaded from memory before execution.

Consider updating tx_script_main.masm to load the transaction script arguments prior to executing the script.

Update: Resolved in pull request #2770. The team stated:

“The way we invoke the script in TransactionExecutor::execute_tx_view_script means the args are always the empty word. So, we documented that the args are always empty.”

Delta Commitment Computation May Use Foreign Account Context

The account_compute_delta_commitment procedure calculates the native account's delta commitment. It invokes account_delta::compute_commitment, which among other fields, hashes the storage slot deltas in a loop bounded by memory::get_num_storage_slots.

However, get_num_storage_slots returns the active account's slot count, while the subsequent account::get_storage_slot_type call accesses memory from the native account section. As a result, if a foreign account is active, the computed delta commitment may be incorrect due to reading incorrect portions of memory.

Consider enforcing the native account context within account_compute_delta_commitment by calling memory::assert_native_account and changing the procedure's name to include "native". Alternatively, ensure that all values read from memory correspond to the native account instead of the active account.

Update: Partially Resolved in pull request #2770. The specific cited defect (active loop bound) is correctly fixed, but compute_delta_commitment still reads the active account for map-slot IDs (commit_map_slot_patch fetches the slot identifier with account::get_slot_id, which resolves via memory::get_account_active_storage_slots_section_ptr) and for the nonce delta (was_nonce_incremented compares the global native init nonce against memory::get_account_nonce, which calls get_active_account_data_ptr). The team stated:

“Introduced memory::get_native_num_storage_slots for use in account_delta::compute_commitment to make sure the delta commitment is always computed against native account state.”

Authentication Does Not Bind Runtime-Computed Fee or Enforce Max Fee

The transaction kernel calculates fees dynamically based on the number of executed VM cycles and a base fee parameter. The compute_fee procedure determines the cost by measuring the current clock cycle, adding a fixed estimate for post-fee work, and applying a logarithmic scaling factor. This design intends to charge users proportionally for the computational resources consumed during transaction execution.

However, the finalize_transaction procedure authenticates the transaction before the final fee is computed or deducted. The standard authentication scheme signs a transaction summary derived from state commitments (inputs, outputs, account delta) but omits the transaction script root or a maximum fee limit. Consequently, a relayer or prover can modify the executed program by substituting a computationally expensive transaction script root that preserves the same state commitments, to artificially inflate the cycle count. Since the fee is derived from the clock cycle at the end of execution, this manipulation increases the cost without invalidating the user's signature, exposing signers to fee griefing or forced failures if the account vault cannot cover the inflated amount.

Consider introducing a signer-controlled fee bound, such as a maximum fee or maximum cycles parameter, that is included in the signed transaction summary and enforced by the kernel before fee deduction. Additionally, consider implementing a syscall that exposes the current fee to the user context.

Update: Acknowledged, will resolve. The team stated:

“We have started discussing how to do this but have not yet implemented it, as we are figuring out how to best do this. Most likely we will include the tx script root in the tx summary commitment.
Issue: https://github.com/0xMiden/protocol/issues/2765”

Misleading Documentation

Throughout the codebase, multiple instances of misleading documentation were identified:

  • In account_get_id, the returned account_id_{prefix,suffix} are not necessarily from the active account; they may also be from the native account.
  • The set_map_item_raw procedure has only 8 locals instead of the documented 12.
  • In lines 339, 345, and 352 of asset_vault.masm, the vault_root_ptr input is missing at the bottom of the stack. Additionally, in line 413, the stack state is missing a zero.
  • In line 537 of account_delta.masm, vault_root_ptr is not actually in the stack.
  • In lines 199 and 240 of faucet.masm, faucet_storage_data_slot and map_slot_index should be replaced with faucet_slot_id_prefix, faucet_slot_id_suffix.
  • In input_note_get_recipient, the input stack should only contain 14 padding elements.
  • In add_fungible_asset, the output stack should also contain note_idx.
  • The decimal number present in ERR_NOTE_FUNGIBLE_MAX_AMOUNT_EXCEEDED and ERR_VAULT_FUNGIBLE_MAX_AMOUNT_EXCEEDED is incorrect. It should be 9223372034707292160 (or 2**63 - 2**31).
  • In lines 239 and 243 of note.masm, current_note_ptr should not be present in the stack, since it was previously moved to the top and consumed.
  • The get_account_data_ptr procedure documentation states the procedure panics if "the prefix OR suffix of the provided foreign account ID equal zero". However, the procedure only panics if both the prefix AND suffix equal zero. Additionally, the procedure relies on a padding element that is not documented as an input, which is unintuitive and error-prone.
  • In line 1108 of account.masm, the order of account_id_prefix and account_id_suffix should be reversed.
  • In line 1831 of account.masm, is_empty_word should be is_empty_block to maintain consistency with adjacent comments.
  • The update_entry documentation states the procedure updates the VALUE0 in the given entry. However, it also updates the VALUE1.
  • In line 531 of link_map.masm, the entry_ptr element at the top of the stack was popped by the previous line.
  • In link_map.masm, the InsertAfterEntry and AbsentAfterEntry operations are said to be represented by the literal 2. However, they are not actually defined as constants like the rest of the operations.
  • In peek_asset, faucet_id_{prefix, suffix} are neither inputs nor outputs of the procedure.
  • In line 1483 of api.masm and in lines 1088 and 1161 of account.masm, the comment describing the storage slot layout is missing the slot ID prefix and suffix.
  • In output_note.masm, from line 503 onward, the first 0 should be replaced by faucet_id_suffix. From line 509 onward, the same applies to the third 0.
  • The MAX_NUM_FOREIGN_ACCOUNTS and ERR_FOREIGN_ACCOUNT_MAX_NUMBER_EXCEEDED constants (alongside this comment) suggest that the maximum number of foreign accounts loaded at a time is 64. However, the actual limit is 63 foreign accounts plus the native account (i.e., 64 is the maximum total number of accounts).
  • The documentation of the adv_pipe instruction states: "writes [D,E] to memory at a and a+1. a' ← a+2". However, this is incorrect, since 2 words (8 field elements) are written to memory.

Consider updating the documentation accordingly to improve the clarity and maintainability of the codebase.

Update: Partially Resolved in pull request #2770. Most instances that were not addressed by the PR are already fixed in the latest commit, except for ERR_NOTE_FUNGIBLE_MAX_AMOUNT_EXCEEDED which still carries the wrong decimal.

Incomplete Procedure Documentation

Throughout the codebase, multiple procedures were found to have incomplete documentation:

Consider completing the documentation to improve clarity and readability across the codebase.

Update: Partially Resolved in pull request #2770. Most instances not fixed by PR #2770 are either resolved in the latest next commit (currently a0ed380) or no longer apply due to codebase restructuring, except for the missing locals documentation in set_map_item.

Notes & Additional Information

get_requested_note_ptr Validates note_index Even When is_active_note = 1

The get_requested_note_ptr helper procedure in api.masm returns the memory pointer to an input note, which can be either the active note or a note requested by index, depending on the is_active_note input. According to the documentation, when is_active_note is 1, the note_index is ignored.

However, input_note::get_input_note_ptr is unconditionally executed within get_requested_note_ptr, causing the note_index to always be validated for bounds, even when the active note is requested.

Consider updating the get_requested_note_ptr implementation and/or its documentation to ensure consistency between the described and actual behavior.

Update: Acknowledged, will resolve. The team stated:

“Thank you and agreed on these improvements! We deem this one low-priority enough to fix it later due to time constraints.”

Inconsistent Caller Authentication in Kernel Procedures

Within api.masm, some kernel procedures enforce that their invocation originates from an account context. However, the criteria used to determine when this check is required is not clearly defined, particularly for read-only kernel procedures.

For example, account_compute_commitment does not authenticate the invocation origin, whereas account_compute_storage_commitment does, even though both procedures are closely related in functionality.

Consider clearly documenting the rationale behind caller validation requirements and applying the chosen criteria consistently across all kernel procedures.

Update: Acknowledged, will resolve. The team stated:

“Agreed that this is inconsistent. We have discussed some of this in https://github.com/0xMiden/protocol/issues/2605. Essentially, the guiding principle should be “Account state (which includes both storage and asset vault) should be accessible only via public account procedures.”, and so all account-related kernel procedures should be accessible only from the account context.”

Typographical Errors

Throughout the codebase, the following typographical errors were identified:

  • In lines 566, 570, and 574 of asset_vault.masm, a comma is missing between the last two stack items.
  • In line 910 of api.masm, there is an extra "not".
  • In lines 42 and 44 of output_note.masm, the casing is inconsistent (e.g., “kind none” vs. “kind None”).

Consider fixing these typographical errors to improve the overall readability of the codebase.

Update: Acknowledged, will resolve. The team stated:

“Thank you and agreed on these improvements! We deem these low-priority enough to fix them later due to time constraints.”

Code Clarity

Throughout the codebase, multiple opportunities for improving code clarity were identified:

  • The 0xffffffff00000000 hex literal used in incr_nonce could be replaced with the FELT_MAX constant defined in account_delta.masm, and the constant could be moved to constants.masm.
  • The literal 4 could be replaced with an ASSET_DATA_LENGTH constant in lines 168, 486, 535, 576, and 595 of output_note.masm.
  • In tx_start_foreign_context, the exec.memory::push_ptr_to_account_stack call could be moved outside the if-else statement, as it is duplicated in both branches.
  • In exec_kernel_proc, the literal 4 could be extracted into a KERNEL_PROCEDURE_DATA_LENGTH constant. Alternatively, the ACCOUNT_PROCEDURE_DATA_LENGTH constant could be reused and renamed for clarity.
  • The OUTPUT_NOTE_HASHING_MEM_DIFF = 2040 constant could be dynamically computed as NOTE_MEM_SIZE - 8.
  • The get_storage_slot_type procedure in account.masm could be renamed to make it clear that it only applies to the native account.

Consider addressing the above points to improve the overall clarity and maintainability of the codebase.

Update: Acknowledged, will resolve. The team stated:

“Thank you and agreed on these improvements! We deem these low-priority enough to fix them later due to time constraints.”

Redundant Operations

In prologue.masm, there is an instance of not followed by assertz, which is equivalent to just using assert. This adds an unnecessary instruction and reduces clarity.

In assert_key_is_equal in link_map.masm, the swapw operation before assert_eqw is redundant, since the equality check does not depend on the order of the operands.

Consider removing the not instruction and replacing assertz with assert in prologue.masm. Additionally, consider removing the swapw instruction in link_map.masm.

Update: Acknowledged, will resolve. The team stated:

“Thank you and agreed on these improvements! We deem these low-priority enough to fix them later due to time constraints.”

Conclusion

The Miden transaction kernel is a critical component for enforcing correct state transitions within the zero-knowledge environment of the Miden blockchain. All transactions in Miden are executed inside the Miden VM, with the transaction kernel responsible for validating and processing each transaction.

Following an initial high-level review, this second review examined all kernel components in greater depth, with particular attention to the correctness of stack and memory operations. During the review, multiple Medium+ severity issues were identified, ranging from insufficient input validation to memory overlaps and arbitrary writes. In addition, several opportunities to improve code and documentation clarity and consistency were noted. Overall, the codebase was found to be well structured and comprehensively documented, enhancing its auditability.

The Miden team is commended for their readiness to provide detailed technical explanations, clarify design decisions, and share comprehensive documentation. These efforts greatly facilitated the assessment and demonstrated a strong commitment to delivering a secure and reliable blockchain protocol.

Appendix

Issue Classification

OpenZeppelin classifies smart contract vulnerabilities on a 5-level scale:

  • Critical
  • High
  • Medium
  • Low
  • Note/Information

Critical Severity

This classification is applied when the issue’s impact is catastrophic, threatening extensive damage to the client's reputation and/or causing severe financial loss to the client or users. The likelihood of exploitation can be high, warranting a swift response. Critical issues typically involve significant risks such as the permanent loss or locking of a large volume of users' sensitive assets or the failure of core system functionalities without viable mitigations. These issues demand immediate attention due to their potential to compromise system integrity or user trust significantly.

High Severity

These issues are characterized by the potential to substantially impact the client’s reputation and/or result in considerable financial losses. The likelihood of exploitation is significant, warranting a swift response. Such issues might include temporary loss or locking of a significant number of users' sensitive assets or disruptions to critical system functionalities, albeit with potential, yet limited, mitigations available. The emphasis is on the significant but not always catastrophic effects on system operation or asset security, necessitating prompt and effective remediation.

Medium Severity

Issues classified as being of medium severity can lead to a noticeable negative impact on the client's reputation and/or moderate financial losses. Such issues, if left unattended, have a moderate likelihood of being exploited or may cause unwanted side effects in the system. These issues are typically confined to a smaller subset of users' sensitive assets or might involve deviations from the specified system design that, while not directly financial in nature, compromise system integrity or user experience. The focus here is on issues that pose a real but contained risk, warranting timely attention to prevent escalation.

Low Severity

Low-severity issues are those that have a low impact on the client's operations and/or reputation. These issues may represent minor risks or inefficiencies to the client's specific business model. They are identified as areas for improvement that, while not urgent, could enhance the security and quality of the codebase if addressed.

Notes & Additional Information Severity

This category is reserved for issues that, despite having a minimal impact, are still important to resolve. Addressing these issues contributes to the overall security posture and code quality improvement but does not require immediate action. It reflects a commitment to maintaining high standards and continuous improvement, even in areas that do not pose immediate risks.