fix(auth): reject AuthRequest events emitted outside the auth procedure

CHANGELOG.md

@@ -2,6 +2,7 @@
 ## v0.16.0 (TBD)
 
 ### Changes
+- Fixed the transaction executor host honoring `AuthRequest` events emitted outside the registered auth procedure, which let untrusted note or transaction scripts force the host to sign; signature production is now restricted to the authentication procedure ([#3233](https://github.com/0xMiden/protocol/pull/3233)).
 - Added the `miden::protocol::tx::compute_fee` procedure, which lets account and note code compute the transaction fee during execution ([#3211](https://github.com/0xMiden/protocol/issues/3211)).
 - [BREAKING] Refactored RBAC role administration to be fully role-based, removing the `Ownable2Step` owner as an unconditional super-admin over the role graph. Replaced `RoleBasedAccessControl::empty()` with `RoleBasedAccessControl::new(initial_admin)` / `with_admins(..)` (which seed the `ADMIN` role), and renamed the `ERR_SENDER_NOT_OWNER_OR_ROLE_ADMIN` abort to `ERR_SENDER_NOT_ROLE_ADMIN` ([#3215](https://github.com/0xMiden/protocol/pull/3215)).
 - Re-exported `LoadedMastForest` from `miden-tx` so consumers implementing the re-exported `MastForestStore` trait can name its return type without depending on `miden-processor` directly ([#3236](https://github.com/0xMiden/protocol/pull/3237)).

crates/miden-testing/src/kernel_tests/tx/test_auth.rs

@@ -1,4 +1,6 @@
 use anyhow::Context;
+use assert_matches::assert_matches;
+use miden_protocol::account::auth::AuthScheme;
 use miden_protocol::account::{Account, AccountBuilder};
 use miden_protocol::errors::MasmError;
 use miden_protocol::errors::tx_kernel::ERR_EPILOGUE_AUTH_PROCEDURE_CALLED_FROM_WRONG_CONTEXT;
@@ -7,9 +9,11 @@ use miden_protocol::{Felt, ONE};
 use miden_standards::account::wallets::BasicWallet;
 use miden_standards::code_builder::CodeBuilder;
 use miden_standards::testing::account_component::{ConditionalAuthComponent, ERR_WRONG_ARGS_MSG};
+use miden_standards::testing::account_interface::get_public_keys_from_account;
 use miden_standards::testing::mock_account::MockAccountExt;
+use miden_tx::TransactionExecutorError;
 
-use crate::{Auth, TestTransactionBuilder, assert_transaction_executor_error};
+use crate::{Auth, MockChain, TestTransactionBuilder, assert_transaction_executor_error};
 
 pub const ERR_WRONG_ARGS: MasmError = MasmError::from_static_str(ERR_WRONG_ARGS_MSG);
 
@@ -97,3 +101,61 @@ async fn test_auth_procedure_called_from_wrong_context() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+/// Regression test: an untrusted transaction script must not be able to force the host to produce a
+/// signature.
+#[tokio::test]
+async fn test_auth_request_from_script_is_rejected() -> anyhow::Result<()> {
+    let mut builder = MockChain::builder();
+    let account = builder.add_existing_mock_account(Auth::BasicAuth {
+        auth_scheme: AuthScheme::Falcon512Poseidon2,
+    })?;
+    let chain = builder.build()?;
+
+    let pub_keys = get_public_keys_from_account(&account);
+    let pub_key = pub_keys.first().expect("expected at least one public key");
+
+    // Mirror `auth::authenticate_transaction`'s signature request, minus the nonce increment.
+    let tx_script_source = format!(
+        "
+        use miden::standards::auth
+        use {{AUTH_REQUEST_EVENT}} from miden::protocol::auth
+
+        @transaction_script
+        pub proc main
+            # [PK_COMM, scheme_id] mirrors the auth procedure's inputs
+            push.2
+            push.{pub_key}
+            # arbitrary salt
+            push.0.0.0.0
+            # => [SALT, PK_COMM, scheme_id]
+
+            exec.auth::create_tx_summary
+            adv.insert_hqword
+            exec.auth::hash_tx_summary
+            # => [MESSAGE, PK_COMM, scheme_id]
+
+            emit.AUTH_REQUEST_EVENT
+
+            # unreachable once the request is rejected; keeps the script well-formed
+            dropw dropw drop
+        end
+        "
+    );
+
+    let tx_script = CodeBuilder::new().compile_tx_script(&tx_script_source)?;
+
+    let execution_result = chain
+        .build_tx_context(account.id(), &[], &[])?
+        .tx_script(tx_script)
+        .build()?
+        .execute()
+        .await;
+
+    assert_matches!(
+        execution_result,
+        Err(TransactionExecutorError::AuthRequestOutsideAuthProcedure)
+    );
+
+    Ok(())
+}

crates/miden-tx/src/errors/mod.rs

@@ -133,6 +133,8 @@ pub enum TransactionExecutorError {
         "failed to respond to signature requested since no authenticator is assigned to the host"
     )]
     MissingAuthenticator,
+    #[error("received an auth request event emitted outside the authentication procedure")]
+    AuthRequestOutsideAuthProcedure,
 }
 
 #[cfg(any(test, feature = "testing"))]
@@ -208,6 +210,8 @@ pub enum TransactionKernelError {
         "failed to respond to signature requested since no authenticator is assigned to the host"
     )]
     MissingAuthenticator,
+    #[error("received an auth request event emitted outside the authentication procedure")]
+    AuthRequestOutsideAuthProcedure,
     #[error("failed to generate signature")]
     SignatureGenerationFailed(#[source] AuthenticationError),
     #[error("transaction returned unauthorized event but a commitment did not match: {0}")]

crates/miden-tx/src/executor/exec_host.rs

@@ -97,6 +97,15 @@ where
     /// authenticator that produced it.
     generated_signatures: BTreeMap<Word, Vec<Felt>>,
 
+    /// Whether execution is currently inside the authentication procedure.
+    ///
+    /// The epilogue wraps the auth procedure between the `EpilogueAuthProcStart` and
+    /// `EpilogueAuthProcEnd` events, so this flag is `true` only while the registered auth
+    /// procedure is running. It is used to reject `AuthRequest` events emitted from any other
+    /// context (e.g. untrusted note or transaction scripts), which must never trigger signature
+    /// production.
+    in_auth_procedure: bool,
+
     /// The source manager to track source code file span information, improving any MASM related
     /// error messages.
     source_manager: Arc<dyn SourceManagerSync>,
@@ -138,6 +147,7 @@ where
             accessed_foreign_account_code: Vec::new(),
             foreign_account_slot_names: BTreeMap::new(),
             generated_signatures: BTreeMap::new(),
+            in_auth_procedure: false,
             source_manager,
         }
     }
@@ -584,13 +594,22 @@ where
                 TransactionEvent::AuthRequest {
                     pub_key_commitment,
                     tx_summary_or_signature,
-                } => match tx_summary_or_signature {
-                    TxSummaryOrSignature::Signature(signature) => {
-                        Ok(self.base_host.on_auth_requested(signature))
-                    },
-                    TxSummaryOrSignature::TxSummary(tx_summary) => {
-                        self.on_auth_requested(pub_key_commitment, tx_summary).await
-                    },
+                } => {
+                    // Signature production is only permitted while the registered auth procedure
+                    // is executing. An `AuthRequest` emitted from any other context (e.g. an
+                    // untrusted note or transaction script) must not force the host to sign.
+                    if !self.in_auth_procedure {
+                        Err(TransactionKernelError::AuthRequestOutsideAuthProcedure)
+                    } else {
+                        match tx_summary_or_signature {
+                            TxSummaryOrSignature::Signature(signature) => {
+                                Ok(self.base_host.on_auth_requested(signature))
+                            },
+                            TxSummaryOrSignature::TxSummary(tx_summary) => {
+                                self.on_auth_requested(pub_key_commitment, tx_summary).await
+                            },
+                        }
+                    }
                 },
 
                 // This always returns an error to abort the transaction.
@@ -643,10 +662,12 @@ where
                     },
                     TransactionProgressEvent::EpilogueAuthProcStart(clk) => {
                         self.tx_progress.start_auth_procedure(clk);
+                        self.in_auth_procedure = true;
                         Ok(Vec::new())
                     },
                     TransactionProgressEvent::EpilogueAuthProcEnd(clk) => {
                         self.tx_progress.end_auth_procedure(clk);
+                        self.in_auth_procedure = false;
                         Ok(Vec::new())
                     },
                 },

crates/miden-tx/src/executor/mod.rs

@@ -474,6 +474,8 @@ fn validate_num_cycles(num_cycles: u32) -> Result<(), TransactionExecutorError>
 ///
 /// - If the inner error is [`TransactionKernelError::Unauthorized`], it is remapped to
 ///   [`TransactionExecutorError::Unauthorized`].
+/// - If the inner error is [`TransactionKernelError::AuthRequestOutsideAuthProcedure`], it is
+///   remapped to [`TransactionExecutorError::AuthRequestOutsideAuthProcedure`].
 /// - Otherwise, the execution error is wrapped in
 ///   [`TransactionExecutorError::TransactionProgramExecutionFailed`].
 fn map_execution_error(exec_err: ExecutionError) -> TransactionExecutorError {
@@ -486,6 +488,9 @@ fn map_execution_error(exec_err: ExecutionError) -> TransactionExecutorError {
                 Some(TransactionKernelError::MissingAuthenticator) => {
                     TransactionExecutorError::MissingAuthenticator
                 },
+                Some(TransactionKernelError::AuthRequestOutsideAuthProcedure) => {
+                    TransactionExecutorError::AuthRequestOutsideAuthProcedure
+                },
                 _ => TransactionExecutorError::TransactionProgramExecutionFailed(exec_err),
             }
         },