From 1bd5535c46533c0f062d4bad541394f95699dff9 Mon Sep 17 00:00:00 2001 From: mootz12 Date: Fri, 3 Apr 2026 16:14:05 -0400 Subject: [PATCH 1/3] feat: add strict-auth check --- Cargo.lock | 7 + .../tests/fixtures/test-wasms/auth/Cargo.toml | 17 + .../tests/fixtures/test-wasms/auth/src/lib.rs | 386 +++++++++++++ .../soroban-test/tests/it/integration.rs | 1 + .../soroban-test/tests/it/integration/auth.rs | 119 +++++ .../soroban-test/tests/it/integration/util.rs | 1 + cmd/soroban-cli/src/auth.rs | 505 ++++++++++++++++++ .../src/commands/contract/extend.rs | 3 + .../src/commands/contract/restore.rs | 3 + cmd/soroban-cli/src/lib.rs | 1 + cmd/soroban-cli/src/log/auth.rs | 87 ++- cmd/soroban-cli/src/signer/mod.rs | 6 +- cmd/soroban-cli/src/tx.rs | 7 +- 13 files changed, 1139 insertions(+), 4 deletions(-) create mode 100644 cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/Cargo.toml create mode 100644 cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/src/lib.rs create mode 100644 cmd/crates/soroban-test/tests/it/integration/auth.rs create mode 100644 cmd/soroban-cli/src/auth.rs diff --git a/Cargo.lock b/Cargo.lock index d3394d8cb3..d56db9f17b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5737,6 +5737,13 @@ dependencies = [ "test-case-core", ] +[[package]] +name = "test_auth" +version = "25.2.0" +dependencies = [ + "soroban-sdk", +] + [[package]] name = "test_constructor" version = "25.2.0" diff --git a/cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/Cargo.toml b/cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/Cargo.toml new file mode 100644 index 0000000000..aabcbab5d0 --- /dev/null +++ b/cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "test_auth" +version = "25.2.0" +authors = ["Stellar Development Foundation "] +license = "Apache-2.0" +edition = "2021" +publish = false + +[lib] +crate-type = ["cdylib", "rlib"] +doctest = false + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"]} diff --git a/cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/src/lib.rs b/cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/src/lib.rs new file mode 100644 index 0000000000..4d4f46b64a --- /dev/null +++ b/cmd/crates/soroban-test/tests/fixtures/test-wasms/auth/src/lib.rs @@ -0,0 +1,386 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, vec, Address, Env, IntoVal, Symbol}; + +#[contract] +pub struct AuthContract; + +#[contractimpl] +impl AuthContract { + /// Constructor with auth + pub fn __constructor(_env: Env, addr: Address) { + addr.require_auth(); + } + + /// require_auth on addr + /// + /// Used by other functions to emulate different nested auth options + pub fn do_auth(_e: Env, addr: Address, val: Symbol) -> Symbol { + addr.require_auth(); + val + } + + /// require_auth on `addr` + /// -> `subcall` does require_auth on `addr` + /// + /// Used by other functions to emulate different nested auth options + pub fn auth_sub_auth(e: Env, addr: Address, val: Symbol, subcall: Address) -> Symbol { + addr.require_auth(); + + let fn_symbol = Symbol::new(&e, "do_auth"); + e.invoke_contract::( + &subcall, + &fn_symbol, + vec![&e, addr.into_val(&e), val.into_val(&e)], + ) + } + + /// require_auth on `addr` + /// -> `subcall` does require_auth on `addr` + /// -> `subcall2` does require_auth on `addr` + pub fn auth_sub_nested_auth( + e: Env, + addr: Address, + val: Symbol, + subcall: Address, + subcall2: Address, + ) -> Symbol { + addr.require_auth(); + + let fn_symbol = Symbol::new(&e, "auth_sub_auth"); + e.invoke_contract::( + &subcall, + &fn_symbol, + vec![ + &e, + addr.into_val(&e), + val.into_val(&e), + subcall2.into_val(&e), + ], + ) + } + + /// require_auth_for_args(val) on `addr` + /// -> `subcall` does require_auth on `addr` + pub fn partial_auth_sub_auth(e: Env, addr: Address, val: Symbol, subcall: Address) -> Symbol { + addr.require_auth_for_args(vec![&e, addr.into_val(&e), val.into_val(&e)]); + + let fn_symbol = Symbol::new(&e, "do_auth"); + e.invoke_contract::( + &subcall, + &fn_symbol, + vec![&e, addr.into_val(&e), val.into_val(&e)], + ) + } + + /// require_auth_for_args(1i128, 2i128) on `addr` + /// -> `subcall` does require_auth on `addr` + pub fn diff_auth_sub_auth(e: Env, addr: Address, val: Symbol, subcall: Address) -> Symbol { + addr.require_auth_for_args(vec![&e, 1i128.into_val(&e), 2i128.into_val(&e)]); + + let fn_symbol = Symbol::new(&e, "do_auth"); + e.invoke_contract::( + &subcall, + &fn_symbol, + vec![&e, addr.into_val(&e), val.into_val(&e)], + ) + } + + /// no auth + /// -> `subcall` does require_auth on `addr` + pub fn no_auth_sub_auth(e: Env, addr: Address, val: Symbol, subcall: Address) -> Symbol { + let fn_symbol = Symbol::new(&e, "do_auth"); + e.invoke_contract::( + &subcall, + &fn_symbol, + vec![&e, addr.into_val(&e), val.into_val(&e)], + ) + } + + /// no auth + /// -> `subcall` does require_auth on `addr` + /// -> `subcall2` does require_auth on `addr` + pub fn no_auth_sub_nested_auth( + e: Env, + addr: Address, + val: Symbol, + subcall: Address, + subcall2: Address, + ) -> Symbol { + let fn_symbol = Symbol::new(&e, "auth_sub_auth"); + e.invoke_contract::( + &subcall, + &fn_symbol, + vec![ + &e, + addr.into_val(&e), + val.into_val(&e), + subcall2.into_val(&e), + ], + ) + } +} + +#[cfg(test)] +mod test { + extern crate std; + + use soroban_sdk::{ + testutils::{Address as _, AuthorizedFunction, AuthorizedInvocation}, + Address, Env, IntoVal, Symbol, + }; + + use crate::{AuthContract, AuthContractClient}; + + #[test] + fn test_do_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id = env.register(AuthContract, (user.clone(),)); + let client = AuthContractClient::new(&env, &contract_id); + + let res = client.do_auth(&user, &val); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }, + )] + ); + assert_eq!(res, val); + } + + #[test] + fn test_auth_sub_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id_1 = env.register(AuthContract, (user.clone(),)); + let client_1 = AuthContractClient::new(&env, &contract_id_1); + let contract_id_2 = env.register(AuthContract, (user.clone(),)); + + let res = client_1.auth_sub_auth(&user, &val, &contract_id_2); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_1.clone(), + Symbol::new(&env, "auth_sub_auth"), + (&user, &val, &contract_id_2).into_val(&env), + )), + sub_invocations: std::vec![AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_2.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }], + }, + )] + ); + assert_eq!(res, val); + } + + #[test] + fn test_auth_sub_nested_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id_1 = env.register(AuthContract, (user.clone(),)); + let client_1 = AuthContractClient::new(&env, &contract_id_1); + let contract_id_2 = env.register(AuthContract, (user.clone(),)); + let contract_id_3 = env.register(AuthContract, (user.clone(),)); + + let res = client_1.auth_sub_nested_auth(&user, &val, &contract_id_2, &contract_id_3); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_1.clone(), + Symbol::new(&env, "auth_sub_nested_auth"), + (&user, &val, &contract_id_2, &contract_id_3).into_val(&env), + )), + sub_invocations: std::vec![AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_2.clone(), + Symbol::new(&env, "auth_sub_auth"), + (&user, &val, &contract_id_3).into_val(&env), + )), + sub_invocations: std::vec![AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_3.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }], + }], + }, + )] + ); + assert_eq!(res, val); + } + + #[test] + fn test_partial_auth_sub_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id_1 = env.register(AuthContract, (user.clone(),)); + let client_1 = AuthContractClient::new(&env, &contract_id_1); + let contract_id_2 = env.register(AuthContract, (user.clone(),)); + + let res = client_1.partial_auth_sub_auth(&user, &val, &contract_id_2); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_1.clone(), + Symbol::new(&env, "partial_auth_sub_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_2.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }], + }, + )] + ); + assert_eq!(res, val); + } + + #[test] + fn test_diff_auth_sub_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id_1 = env.register(AuthContract, (user.clone(),)); + let client_1 = AuthContractClient::new(&env, &contract_id_1); + let contract_id_2 = env.register(AuthContract, (user.clone(),)); + + let res = client_1.diff_auth_sub_auth(&user, &val, &contract_id_2); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_1.clone(), + Symbol::new(&env, "diff_auth_sub_auth"), + (&1i128, &2i128).into_val(&env), + )), + sub_invocations: std::vec![AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_2.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }], + }, + )] + ); + assert_eq!(res, val); + } + + #[test] + fn test_no_auth_sub_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id_1 = env.register(AuthContract, (user.clone(),)); + let client_1 = AuthContractClient::new(&env, &contract_id_1); + let contract_id_2 = env.register(AuthContract, (user.clone(),)); + + let res = client_1.no_auth_sub_auth(&user, &val, &contract_id_2); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_2.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }, + )] + ); + assert_eq!(res, val); + } + + #[test] + fn test_no_auth_sub_nested_auth_creates_expected_auth() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + + let user = Address::generate(&env); + let val = Symbol::new(&env, "test_auth"); + + let contract_id_1 = env.register(AuthContract, (user.clone(),)); + let client_1 = AuthContractClient::new(&env, &contract_id_1); + let contract_id_2 = env.register(AuthContract, (user.clone(),)); + let contract_id_3 = env.register(AuthContract, (user.clone(),)); + + let res = client_1.no_auth_sub_nested_auth(&user, &val, &contract_id_2, &contract_id_3); + assert_eq!( + env.auths(), + std::vec![( + user.clone(), + AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_2.clone(), + Symbol::new(&env, "auth_sub_auth"), + (&user, &val, &contract_id_3).into_val(&env), + )), + sub_invocations: std::vec![AuthorizedInvocation { + function: AuthorizedFunction::Contract(( + contract_id_3.clone(), + Symbol::new(&env, "do_auth"), + (&user, &val).into_val(&env), + )), + sub_invocations: std::vec![], + }], + }, + )] + ); + assert_eq!(res, val); + } +} diff --git a/cmd/crates/soroban-test/tests/it/integration.rs b/cmd/crates/soroban-test/tests/it/integration.rs index 7f749a7674..9fa5a46ad9 100644 --- a/cmd/crates/soroban-test/tests/it/integration.rs +++ b/cmd/crates/soroban-test/tests/it/integration.rs @@ -1,3 +1,4 @@ +mod auth; mod auto_build; mod bindings; mod constructor; diff --git a/cmd/crates/soroban-test/tests/it/integration/auth.rs b/cmd/crates/soroban-test/tests/it/integration/auth.rs new file mode 100644 index 0000000000..f5df7dc263 --- /dev/null +++ b/cmd/crates/soroban-test/tests/it/integration/auth.rs @@ -0,0 +1,119 @@ +use soroban_test::TestEnv; + +use super::util::{deploy_contract, extend_contract, new_account, DeployOptions, AUTH}; + +/// Helper to deploy two instances of the auth contract and extend them. +/// Returns (contract_id_1, contract_id_2). +async fn deploy_auth_contracts(sandbox: &TestEnv) -> (String, String) { + let id1 = deploy_contract( + sandbox, + AUTH, + DeployOptions { + salt: Some("0000000000000000000000000000000000000000000000000000000000000001".into()), + ..Default::default() + }, + ) + .await; + extend_contract(sandbox, &id1).await; + + let id2 = deploy_contract( + sandbox, + AUTH, + DeployOptions { + salt: Some("0000000000000000000000000000000000000000000000000000000000000002".into()), + ..Default::default() + }, + ) + .await; + extend_contract(sandbox, &id2).await; + + (id1, id2) +} + +#[tokio::test] +async fn standard_auth_with_separate_signer() { + let sandbox = &TestEnv::new(); + let (id, _) = deploy_auth_contracts(sandbox).await; + new_account(sandbox, "signer"); + + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id) + .arg("--") + .arg("do-auth") + .arg("--addr=signer") + .arg("--val=hello") + .assert() + .success() + .stdout("\"hello\"\n"); +} + +#[tokio::test] +async fn root_auth_with_authorized_subcall() { + let sandbox = &TestEnv::new(); + let (id1, id2) = deploy_auth_contracts(sandbox).await; + + new_account(sandbox, "signer"); + + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id1) + .arg("--") + .arg("auth-sub-auth") + .arg("--addr=signer") + .arg("--val=hello") + .arg(&format!("--subcall={id2}")) + .assert() + .success() + .stdout("\"hello\"\n"); +} + +#[tokio::test] +async fn non_root_auth_subcall_fails() { + let sandbox = &TestEnv::new(); + let (id1, id2) = deploy_auth_contracts(sandbox).await; + + new_account(sandbox, "signer"); + + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id1) + .arg("--") + .arg("no-auth-sub-auth") + .arg("--addr=signer") + .arg("--val=hello") + .arg(&format!("--subcall={id2}")) + .assert() + .failure() + .stderr(predicates::str::contains("Auth, InvalidAction")); +} + +#[tokio::test] +async fn partial_auth_source_account_fails() { + let sandbox = &TestEnv::new(); + let (id1, id2) = deploy_auth_contracts(sandbox).await; + + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id1) + .arg("--") + .arg("partial_auth_sub_auth") + .arg("--addr=test") + .arg("--val=hello") + .arg(&format!("--subcall={id2}")) + .assert() + .failure() + .stderr(predicates::str::contains("Auth, InvalidAction")); +} diff --git a/cmd/crates/soroban-test/tests/it/integration/util.rs b/cmd/crates/soroban-test/tests/it/integration/util.rs index 21eb71874d..826c985e47 100644 --- a/cmd/crates/soroban-test/tests/it/integration/util.rs +++ b/cmd/crates/soroban-test/tests/it/integration/util.rs @@ -5,6 +5,7 @@ use soroban_cli::{ use soroban_test::{AssertExt, TestEnv, Wasm}; use std::fmt::Display; +pub const AUTH: &Wasm = &Wasm::Custom("test-wasms", "test_auth"); pub const HELLO_WORLD: &Wasm = &Wasm::Custom("test-wasms", "test_hello_world"); pub const CONSTRUCTOR: &Wasm = &Wasm::Custom("test-wasms", "test_constructor"); pub const CUSTOM_TYPES: &Wasm = &Wasm::Custom("test-wasms", "test_custom_types"); diff --git a/cmd/soroban-cli/src/auth.rs b/cmd/soroban-cli/src/auth.rs new file mode 100644 index 0000000000..969df0ec8e --- /dev/null +++ b/cmd/soroban-cli/src/auth.rs @@ -0,0 +1,505 @@ +use crate::{ + log::{auth, format_auth_entry}, + print, signer, + xdr::{ + AccountId, HostFunction, InvokeContractArgs, InvokeHostFunctionOp, MuxedAccount, Operation, + OperationBody, PublicKey, ScAddress, SorobanAuthorizationEntry, SorobanAuthorizedFunction, + SorobanCredentials, Transaction, Uint256, + }, +}; + +/// Check transactions for any malformed or non-strict authorization entries. These entries, if signed, +/// could be submitted outside of the context of the transaction's contract invocation. Note that +/// this function does not protect against interacting with a malicious contract, and should not +/// be relied on as protection against potentially malicious transactions. +/// +/// Enforces two checks: +/// +/// 1. **Source account uses SourceAccount credentials** +/// The source account's auth should use `SourceAccount` credentials. If it +/// appears as `Address`, the simulation's auth recording is not correct. +/// +/// 2. **Auth entry root invocation matches the tx operation** +/// Auth entries with `Address` credentials whose root invocation doesn't match the tx's +/// InvokeHostFunction could be used outside of the context of the transaction. +/// This does impact contracts that use `require_auth_for_args` as there is no way +/// to verify the authorization entry can't be recreated with different function arguments. +/// +/// This function also logs all auth entries that are caught by those two flags. +/// +/// # Errors +/// * If the source account credential is used directly instead of as a `SourceAccount` credential. +/// * If non-root auth is detected +pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { + let print = print::Print::new(quiet); + let source_bytes = source_account_bytes(tx); + // only need to check auth if op is `InvokeHostFunction` + let Some(invoke_host_op) = get_op(tx) else { + return Ok(()); + }; + + let mut non_strict_entries: Vec<&SorobanAuthorizationEntry> = Vec::new(); + for (i, entry) in invoke_host_op.auths().enumerate() { + let SorobanCredentials::Address(ref creds) = entry.credentials else { + // SourceAccount credential entries are not signed explicitly + // so there is no risk of them being used outside the context of the transaction. + continue; + }; + + // Check if source account appears as Address credential + if let Some(auth_addr) = auth_address_bytes(&creds.address) { + if source_bytes == auth_addr { + print.warnln("Source account detected with Address credentials. This requires an extra signature and is not expected."); + print.warnln(auth::format_auth_entry(entry, i)); + return Err(signer::Error::InvalidAuthEntry); + } + } + + // Check if the auth entry is strict. That is, it cannot be submitted outside the + // context of the transaction's host function. This check is overly strict as it doesn't + // allow for the usage of `require_auth_for_args`. + let is_strict = match &invoke_host_op.host_function { + // For `InvokeContract`, validate the root invocation contract and function name match. + HostFunction::InvokeContract(op) => match &entry.root_invocation.function { + SorobanAuthorizedFunction::ContractFn(auth_args) => auth_args == op, + _ => false, + }, + // For `CreateContract` and `CreateContractV2`, the root invocation should + // match the host function arguments. + HostFunction::CreateContract(op) => match &entry.root_invocation.function { + SorobanAuthorizedFunction::CreateContractHostFn(auth_args) => auth_args == op, + _ => false, + }, + HostFunction::CreateContractV2(op) => match &entry.root_invocation.function { + SorobanAuthorizedFunction::CreateContractV2HostFn(auth_args) => auth_args == op, + _ => false, + }, + // auth entries shouldn't exist for other host functions + HostFunction::UploadContractWasm(_) => { + print.warnln(format!( + "Auth entry not expected for the host function {}", + invoke_host_op.host_function.name() + )); + print.warnln(auth::format_auth_entry(entry, i)); + return Err(signer::Error::InvalidAuthEntry); + } + }; + + if !is_strict { + non_strict_entries.push(entry); + } + } + + if non_strict_entries.is_empty() { + Ok(()) + } else { + print.warnln( + "Authorization entries detected that could be submitted outside the context of this transaction:", + ); + for (i, entry) in non_strict_entries.iter().enumerate() { + print.println(format_auth_entry(entry, i)); + } + Err(signer::Error::OutOfContextAuthEntry) + } +} + +/// Extract the Ed25519 public key bytes from a transaction's source account. +fn source_account_bytes(tx: &Transaction) -> [u8; 32] { + match &tx.source_account { + MuxedAccount::Ed25519(Uint256(bytes)) => *bytes, + MuxedAccount::MuxedEd25519(muxed) => muxed.ed25519.0, + } +} + +/// Extract the Ed25519 public key bytes from an auth entry's credential address. +fn auth_address_bytes(address: &ScAddress) -> Option<[u8; 32]> { + match address { + ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(bytes)))) => { + Some(*bytes) + } + _ => None, + } +} + +/// Extract the host function from a transaction, if it's an InvokeHostFunction operation. +fn get_op(tx: &Transaction) -> Option<&InvokeHostFunctionOp> { + let [Operation { + body: OperationBody::InvokeHostFunction(invoke_host_function_op), + .. + }] = tx.operations.as_slice() + else { + return None; + }; + Some(invoke_host_function_op) +} + +/// Extract the function name from a root invocation, if it's a contract function call. +pub fn invocation_function_name(auth: &SorobanAuthorizationEntry) -> Option { + match &auth.root_invocation.function { + SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { function_name, .. }) => { + std::str::from_utf8(function_name.as_ref()) + .ok() + .map(String::from) + } + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::xdr::{ + BytesM, ContractExecutable, ContractIdPreimage, ContractIdPreimageFromAddress, + CreateContractArgsV2, Hash, InvokeContractArgs, InvokeHostFunctionOp, Memo, Preconditions, + ScSymbol, ScVal, SequenceNumber, SorobanAddressCredentials, SorobanAuthorizedFunction, + SorobanAuthorizedInvocation, TransactionExt, VecM, + }; + use stellar_strkey::ed25519; + + const SOURCE_ACCOUNT: &str = "GBZXN7PIRZGNMHGA7MUUUF4GWPY5AYPV6LY4UV2GL6VJGIQRXFDNMADI"; + const OTHER_ACCOUNT: &str = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; + + fn source_account_bytes() -> [u8; 32] { + ed25519::PublicKey::from_string(SOURCE_ACCOUNT).unwrap().0 + } + + fn other_account_bytes() -> [u8; 32] { + ed25519::PublicKey::from_string(OTHER_ACCOUNT).unwrap().0 + } + + fn test_transaction( + host_fn: &HostFunction, + auth_entries: &[SorobanAuthorizationEntry], + ) -> Transaction { + Transaction { + source_account: MuxedAccount::Ed25519(Uint256(source_account_bytes())), + fee: 100, + seq_num: SequenceNumber(1), + cond: Preconditions::None, + memo: Memo::None, + operations: vec![Operation { + source_account: None, + body: OperationBody::InvokeHostFunction(InvokeHostFunctionOp { + host_function: host_fn.clone(), + auth: auth_entries.to_vec().try_into().unwrap(), + }), + }] + .try_into() + .unwrap(), + ext: TransactionExt::V0, + } + } + + fn make_host_fn_invoke_contract( + contract_addr: [u8; 32], + fn_name: &str, + args: &[ScVal], + ) -> HostFunction { + HostFunction::InvokeContract(InvokeContractArgs { + contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash( + contract_addr, + ))), + function_name: ScSymbol(fn_name.try_into().unwrap()), + args: args.try_into().unwrap(), + }) + } + + fn make_host_fn_create_contract(wasm_hash: [u8; 32], args: &[ScVal]) -> HostFunction { + HostFunction::CreateContractV2(CreateContractArgsV2 { + contract_id_preimage: ContractIdPreimage::Address(ContractIdPreimageFromAddress { + address: ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256( + source_account_bytes(), + )))), + salt: Uint256([0u8; 32]), + }), + executable: ContractExecutable::Wasm(wasm_hash.into()), + constructor_args: args.try_into().unwrap(), + }) + } + + fn make_auth_entry( + address_bytes: [u8; 32], + invocation: &SorobanAuthorizedInvocation, + ) -> SorobanAuthorizationEntry { + SorobanAuthorizationEntry { + credentials: SorobanCredentials::Address(SorobanAddressCredentials { + address: ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256( + address_bytes, + )))), + nonce: 0, + signature_expiration_ledger: 0, + signature: ScVal::Void, + }), + root_invocation: invocation.clone(), + } + } + + fn make_source_account_auth_entry( + invocation: &SorobanAuthorizedInvocation, + ) -> SorobanAuthorizationEntry { + SorobanAuthorizationEntry { + credentials: SorobanCredentials::SourceAccount, + root_invocation: invocation.clone(), + } + } + + fn make_auth_invocation_contract( + contract_addr: [u8; 32], + fn_name: &str, + args: &[ScVal], + ) -> SorobanAuthorizedInvocation { + SorobanAuthorizedInvocation { + function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { + contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash( + contract_addr, + ))), + function_name: ScSymbol(fn_name.try_into().unwrap()), + args: args.to_vec().try_into().unwrap(), + }), + sub_invocations: VecM::default(), + } + } + + fn make_auth_invocation_create_contract( + wasm_hash: [u8; 32], + args: &[ScVal], + ) -> SorobanAuthorizedInvocation { + SorobanAuthorizedInvocation { + function: SorobanAuthorizedFunction::CreateContractV2HostFn(CreateContractArgsV2 { + contract_id_preimage: ContractIdPreimage::Address(ContractIdPreimageFromAddress { + address: ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519( + Uint256(source_account_bytes()), + ))), + salt: Uint256([0u8; 32]), + }), + executable: ContractExecutable::Wasm(wasm_hash.into()), + constructor_args: args.try_into().unwrap(), + }), + sub_invocations: VecM::default(), + } + } + + #[test] + fn test_matching_root_invocation_passes() { + let contract = [1u8; 32]; + let other_contract = [99u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + let mut invocation = make_auth_invocation_contract(contract, "hello", args); + let sub_invocation = make_auth_invocation_contract(other_contract, "other", &[]); + invocation.sub_invocations = [sub_invocation].try_into().unwrap(); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_matching_root_invocation_with_subinvocations_passes() { + let contract = [1u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + let invocation = make_auth_invocation_contract(contract, "hello", args); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_source_account_as_address_credential_is_rejected() { + let contract = [1u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + let invocation = make_auth_invocation_contract(contract, "hello", args); + let auth = make_auth_entry(source_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } + + #[test] + fn test_source_account_credentials_passes() { + let contract = [1u8; 32]; + let other_contract = [99u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + let invocation = make_auth_invocation_contract(other_contract, "other", &[]); + let auth = make_source_account_auth_entry(&invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_non_root_auth_is_rejected() { + let contract = [1u8; 32]; + let other_contract = [99u8; 32]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", &[]); + let invocation = make_auth_invocation_contract(other_contract, "hello", &[]); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } + + #[test] + fn test_different_function_same_contract_is_rejected() { + let contract = [1u8; 32]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", &[]); + let invocation = make_auth_invocation_contract(contract, "transfer", &[]); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } + + #[test] + fn test_different_args_is_rejected() { + let contract = [1u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + let wrong_args = &[ScVal::U32(43), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + let invocation = make_auth_invocation_contract(contract, "hello", wrong_args); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } + + #[test] + fn test_multiple_valid_passes() { + let contract = [1u8; 32]; + let other_contract = [99u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + + // Root-matching auth — safe + let root_invocation = make_auth_invocation_contract(contract, "hello", args); + let safe_auth = make_auth_entry(other_account_bytes(), &root_invocation); + + // SourceAccount auth for different contract — safe + let other_invocation = make_auth_invocation_contract(other_contract, "anything", &[]); + let source_auth = make_source_account_auth_entry(&other_invocation); + + // Root auth for different contract + let other_invocation = make_auth_invocation_contract(contract, "hello", args); + let other_auth = make_auth_entry(other_account_bytes(), &other_invocation); + + let tx = test_transaction(&host_fn, &[safe_auth, source_auth, other_auth]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_rejects_if_any_fail_check() { + let contract = [1u8; 32]; + let other_contract = [99u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_invoke_contract(contract, "hello", args); + + // Root-matching auth — safe + let root_invocation = make_auth_invocation_contract(contract, "hello", args); + let safe_auth = make_auth_entry(other_account_bytes(), &root_invocation); + + // SourceAccount auth for different contract — safe + let other_invocation = make_auth_invocation_contract(other_contract, "anything", &[]); + let source_auth = make_source_account_auth_entry(&other_invocation); + + // Non-Root auth for different contract + let other_invocation = make_auth_invocation_contract(other_contract, "hello", args); + let other_auth = make_auth_entry(other_account_bytes(), &other_invocation); + + let tx = test_transaction(&host_fn, &[safe_auth, source_auth, other_auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } + + #[test] + fn test_upload_wasm_host_function_passes() { + let wasm_hash: BytesM = [42u8; 32].try_into().unwrap(); + + let host_fn = HostFunction::UploadContractWasm(wasm_hash); + let tx = test_transaction(&host_fn, &[]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_upload_wasm_host_function_with_auth_is_rejected() { + let contract = [1u8; 32]; + let wasm_hash: BytesM = [42u8; 32].try_into().unwrap(); + + let host_fn = HostFunction::UploadContractWasm(wasm_hash); + let invocation = make_auth_invocation_contract(contract, "hello", &[]); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } + + #[test] + fn test_deploy_no_constructor_passes() { + let wasm_hash = [42u8; 32]; + + let host_fn = make_host_fn_create_contract(wasm_hash, &[]); + let tx = test_transaction(&host_fn, &[]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_deploy_constructor_passes() { + let contract = [1u8; 32]; + let wasm_hash = [42u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_create_contract(wasm_hash, args); + let mut invocation = make_auth_invocation_create_contract(wasm_hash, args); + let sub_invocation = make_auth_invocation_contract(contract, "__constructor", args); + invocation.sub_invocations = [sub_invocation].try_into().unwrap(); + let auth = make_auth_entry(other_account_bytes(), &invocation); + let tx = test_transaction(&host_fn, &[auth]); + + let result = check_auth(&tx, true); + assert!(result.is_ok()); + } + + #[test] + fn test_deploy_constructor_with_non_source_auth() { + let contract = [1u8; 32]; + let wasm_hash = [42u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = make_host_fn_create_contract(wasm_hash, args); + let invocation = make_auth_invocation_create_contract(wasm_hash, args); + let auth = make_auth_entry(source_account_bytes(), &invocation); + let other_invocation = make_auth_invocation_contract(contract, "__constructor", args); + let other_auth = make_auth_entry(other_account_bytes(), &other_invocation); + let tx = test_transaction(&host_fn, &[auth, other_auth]); + + let result = check_auth(&tx, true); + assert!(result.is_err()); + } +} diff --git a/cmd/soroban-cli/src/commands/contract/extend.rs b/cmd/soroban-cli/src/commands/contract/extend.rs index 3ae6aaad41..7f58469be6 100644 --- a/cmd/soroban-cli/src/commands/contract/extend.rs +++ b/cmd/soroban-cli/src/commands/contract/extend.rs @@ -196,6 +196,9 @@ impl Cmd { tracing::trace!(?network); let keys = self.key.parse_keys(&config.locator, &network)?; let client = network.rpc_client()?; + client + .verify_network_passphrase(Some(&network.network_passphrase)) + .await?; let source_account = config.source_account().await?; let extend_to = self.ledgers_to_extend(&client).await?; diff --git a/cmd/soroban-cli/src/commands/contract/restore.rs b/cmd/soroban-cli/src/commands/contract/restore.rs index 54d72abf10..161aa99412 100644 --- a/cmd/soroban-cli/src/commands/contract/restore.rs +++ b/cmd/soroban-cli/src/commands/contract/restore.rs @@ -165,6 +165,9 @@ impl Cmd { tracing::trace!(?network); let entry_keys = self.key.parse_keys(&config.locator, &network)?; let client = network.rpc_client()?; + client + .verify_network_passphrase(Some(&network.network_passphrase)) + .await?; let source_account = config.source_account().await?; // Get the account sequence number diff --git a/cmd/soroban-cli/src/lib.rs b/cmd/soroban-cli/src/lib.rs index a6fd4b26da..4b352ad2c6 100644 --- a/cmd/soroban-cli/src/lib.rs +++ b/cmd/soroban-cli/src/lib.rs @@ -12,6 +12,7 @@ mod cli; pub use cli::main; pub mod assembled; +pub mod auth; pub mod color; pub mod commands; pub mod config; diff --git a/cmd/soroban-cli/src/log/auth.rs b/cmd/soroban-cli/src/log/auth.rs index 4a6b4bea84..345897232f 100644 --- a/cmd/soroban-cli/src/log/auth.rs +++ b/cmd/soroban-cli/src/log/auth.rs @@ -1,7 +1,92 @@ -use crate::xdr::{SorobanAuthorizationEntry, VecM}; +use std::fmt::Write; +use crate::xdr::{ + AccountId, InvokeContractArgs, PublicKey, ScAddress, SorobanAuthorizationEntry, + SorobanAuthorizedFunction, SorobanAuthorizedInvocation, SorobanCredentials, Uint256, VecM, +}; + +/// Legacy debug logging function (preserved for backward compatibility via `pub use auth::*`) pub fn auth(auth: &[VecM]) { if !auth.is_empty() { tracing::debug!("{auth:#?}"); } } + +/// Format a single auth entry for display. +pub fn format_auth_entry(entry: &SorobanAuthorizationEntry, index: usize) -> String { + let mut result = format!(" Auth Entry #{index}:\n"); + + match &entry.credentials { + SorobanCredentials::Address(creds) => { + let _ = writeln!(result, " Signer: {}", format_address(&creds.address)); + } + SorobanCredentials::SourceAccount => { + result.push_str(" Signer: \n"); + } + } + + format_invocation(&entry.root_invocation, 2, 0, &mut result); + + result +} + +/// Recursively format a `SorobanAuthorizedInvocation` tree. +fn format_invocation( + invocation: &SorobanAuthorizedInvocation, + indent: usize, + index: usize, + result: &mut String, +) { + let prefix = " ".repeat(indent); + + match &invocation.function { + SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { + contract_address, + function_name, + args, + }) => { + let fn_name = std::str::from_utf8(function_name.as_ref()).unwrap_or(""); + let _ = writeln!(result, "{prefix}Function #{index}:"); + let _ = writeln!( + result, + "{prefix} Contract: {}", + format_address(contract_address) + ); + let _ = writeln!(result, "{prefix} Fn: {fn_name}"); + if !args.is_empty() { + let _ = writeln!(result, "{prefix} Args:"); + for arg in args.iter() { + let _ = writeln!( + result, + "{prefix} {}", + soroban_spec_tools::to_string(arg) + .unwrap_or(String::from("")) + ); + } + } + } + SorobanAuthorizedFunction::CreateContractHostFn(_) + | SorobanAuthorizedFunction::CreateContractV2HostFn(_) => { + let _ = writeln!(result, "{prefix}Function #{index}:"); + let _ = writeln!(result, "{prefix} CreateContract"); + } + } + + for (i, sub) in invocation.sub_invocations.iter().enumerate() { + format_invocation(sub, indent + 1, i, result); + } +} + +/// Format an ScAddress as a strkey string for display. +fn format_address(address: &ScAddress) -> String { + match address { + ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(bytes)))) => { + stellar_strkey::Strkey::PublicKeyEd25519(stellar_strkey::ed25519::PublicKey(*bytes)) + .to_string() + } + ScAddress::Contract(stellar_xdr::curr::ContractId(stellar_xdr::curr::Hash(bytes))) => { + stellar_strkey::Strkey::Contract(stellar_strkey::Contract(*bytes)).to_string() + } + _ => format!("{address:?}"), + } +} diff --git a/cmd/soroban-cli/src/signer/mod.rs b/cmd/soroban-cli/src/signer/mod.rs index 7482fcf94b..1704e0dbea 100644 --- a/cmd/soroban-cli/src/signer/mod.rs +++ b/cmd/soroban-cli/src/signer/mod.rs @@ -30,8 +30,10 @@ pub enum Error { MissingSignerForAddress { address: String }, #[error(transparent)] TryFromSlice(#[from] std::array::TryFromSliceError), - #[error("User cancelled signing, perhaps need to add -y")] - UserCancelledSigning, + #[error("Signing authorization entries that could be submitted outside the context of the transaction is not supported in the CLI")] + OutOfContextAuthEntry, + #[error("Invalid Soroban authorization entry")] + InvalidAuthEntry, #[error(transparent)] Xdr(#[from] xdr::Error), #[error("Transaction envelope type not supported")] diff --git a/cmd/soroban-cli/src/tx.rs b/cmd/soroban-cli/src/tx.rs index 8aa3a9c373..7d3a543103 100644 --- a/cmd/soroban-cli/src/tx.rs +++ b/cmd/soroban-cli/src/tx.rs @@ -1,5 +1,6 @@ use crate::{ assembled::simulate_and_assemble_transaction, + auth::check_auth, commands::tx::fetch, config::{self, data, network}, print, resources, @@ -65,7 +66,11 @@ where data::write(sim_res.clone().into(), &network.rpc_uri()?)?; } - // Need to sign all auth entries + // Check for malformed, or non-strict auth entries that could be submitted + // outside the context of this transaction + check_auth(&txn, quiet).map_err(config::Error::from)?; + + // Sign all auth entries now that they've been validated/approved if let Some(tx) = config .sign_soroban_authorizations(&txn, auth_signers) .await? From d02a282a652097469dc0b6439972f7dcd6823830 Mon Sep 17 00:00:00 2001 From: mootz12 Date: Tue, 7 Apr 2026 09:23:33 -0400 Subject: [PATCH 2/3] chore: fix comment and misleading entry counter --- cmd/soroban-cli/src/auth.rs | 12 ++++++------ cmd/soroban-cli/src/log/auth.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/soroban-cli/src/auth.rs b/cmd/soroban-cli/src/auth.rs index 969df0ec8e..d6c0ad64f0 100644 --- a/cmd/soroban-cli/src/auth.rs +++ b/cmd/soroban-cli/src/auth.rs @@ -39,7 +39,7 @@ pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { }; let mut non_strict_entries: Vec<&SorobanAuthorizationEntry> = Vec::new(); - for (i, entry) in invoke_host_op.auths().enumerate() { + for entry in invoke_host_op.auths() { let SorobanCredentials::Address(ref creds) = entry.credentials else { // SourceAccount credential entries are not signed explicitly // so there is no risk of them being used outside the context of the transaction. @@ -50,7 +50,7 @@ pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { if let Some(auth_addr) = auth_address_bytes(&creds.address) { if source_bytes == auth_addr { print.warnln("Source account detected with Address credentials. This requires an extra signature and is not expected."); - print.warnln(auth::format_auth_entry(entry, i)); + print.warnln(format_auth_entry(entry)); return Err(signer::Error::InvalidAuthEntry); } } @@ -59,7 +59,7 @@ pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { // context of the transaction's host function. This check is overly strict as it doesn't // allow for the usage of `require_auth_for_args`. let is_strict = match &invoke_host_op.host_function { - // For `InvokeContract`, validate the root invocation contract and function name match. + // For `InvokeContract`, validate the root invocation matches the host function arguments. HostFunction::InvokeContract(op) => match &entry.root_invocation.function { SorobanAuthorizedFunction::ContractFn(auth_args) => auth_args == op, _ => false, @@ -80,7 +80,7 @@ pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { "Auth entry not expected for the host function {}", invoke_host_op.host_function.name() )); - print.warnln(auth::format_auth_entry(entry, i)); + print.warnln(auth::format_auth_entry(entry)); return Err(signer::Error::InvalidAuthEntry); } }; @@ -96,8 +96,8 @@ pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { print.warnln( "Authorization entries detected that could be submitted outside the context of this transaction:", ); - for (i, entry) in non_strict_entries.iter().enumerate() { - print.println(format_auth_entry(entry, i)); + for entry in non_strict_entries { + print.println(format_auth_entry(entry)); } Err(signer::Error::OutOfContextAuthEntry) } diff --git a/cmd/soroban-cli/src/log/auth.rs b/cmd/soroban-cli/src/log/auth.rs index 345897232f..910335691e 100644 --- a/cmd/soroban-cli/src/log/auth.rs +++ b/cmd/soroban-cli/src/log/auth.rs @@ -13,8 +13,8 @@ pub fn auth(auth: &[VecM]) { } /// Format a single auth entry for display. -pub fn format_auth_entry(entry: &SorobanAuthorizationEntry, index: usize) -> String { - let mut result = format!(" Auth Entry #{index}:\n"); +pub fn format_auth_entry(entry: &SorobanAuthorizationEntry) -> String { + let mut result = format!(" Auth Entry:\n"); match &entry.credentials { SorobanCredentials::Address(creds) => { From 865c0e6747e98a88f280991c3b9483e505578f75 Mon Sep 17 00:00:00 2001 From: mootz12 Date: Wed, 29 Apr 2026 12:28:32 -0400 Subject: [PATCH 3/3] feat: add auth entry validation only for entries we expect to sign --- .../soroban-test/tests/it/integration/auth.rs | 104 +++- cmd/soroban-cli/src/auth.rs | 505 ------------------ cmd/soroban-cli/src/lib.rs | 1 - cmd/soroban-cli/src/log/auth.rs | 25 +- cmd/soroban-cli/src/signer/mod.rs | 279 +++++++++- cmd/soroban-cli/src/signer/validation.rs | 222 ++++++++ cmd/soroban-cli/src/tx.rs | 8 +- 7 files changed, 579 insertions(+), 565 deletions(-) delete mode 100644 cmd/soroban-cli/src/auth.rs create mode 100644 cmd/soroban-cli/src/signer/validation.rs diff --git a/cmd/crates/soroban-test/tests/it/integration/auth.rs b/cmd/crates/soroban-test/tests/it/integration/auth.rs index f5df7dc263..e36cc9bbbb 100644 --- a/cmd/crates/soroban-test/tests/it/integration/auth.rs +++ b/cmd/crates/soroban-test/tests/it/integration/auth.rs @@ -1,30 +1,31 @@ -use soroban_test::TestEnv; +use assert_cmd::Command; +use soroban_test::{AssertExt, TestEnv}; -use super::util::{deploy_contract, extend_contract, new_account, DeployOptions, AUTH}; +use super::util::{extend_contract, new_account, AUTH}; + +fn constructor_cmd(sandbox: &TestEnv, addr: &str) -> Command { + let mut cmd = sandbox.new_assert_cmd("contract"); + cmd.arg("deploy") + .arg("--source=test") + .arg("--wasm") + .arg(AUTH.path()); + cmd.arg("--").arg("--addr").arg(addr); + cmd +} /// Helper to deploy two instances of the auth contract and extend them. /// Returns (contract_id_1, contract_id_2). async fn deploy_auth_contracts(sandbox: &TestEnv) -> (String, String) { - let id1 = deploy_contract( - sandbox, - AUTH, - DeployOptions { - salt: Some("0000000000000000000000000000000000000000000000000000000000000001".into()), - ..Default::default() - }, - ) - .await; + let id1 = constructor_cmd(sandbox, "test") + .assert() + .success() + .stdout_as_str(); extend_contract(sandbox, &id1).await; - let id2 = deploy_contract( - sandbox, - AUTH, - DeployOptions { - salt: Some("0000000000000000000000000000000000000000000000000000000000000002".into()), - ..Default::default() - }, - ) - .await; + let id2 = constructor_cmd(sandbox, "test") + .assert() + .success() + .stdout_as_str(); extend_contract(sandbox, &id2).await; (id1, id2) @@ -33,9 +34,10 @@ async fn deploy_auth_contracts(sandbox: &TestEnv) -> (String, String) { #[tokio::test] async fn standard_auth_with_separate_signer() { let sandbox = &TestEnv::new(); - let (id, _) = deploy_auth_contracts(sandbox).await; new_account(sandbox, "signer"); + let (id, _) = deploy_auth_contracts(sandbox).await; + sandbox .new_assert_cmd("contract") .arg("invoke") @@ -54,10 +56,10 @@ async fn standard_auth_with_separate_signer() { #[tokio::test] async fn root_auth_with_authorized_subcall() { let sandbox = &TestEnv::new(); - let (id1, id2) = deploy_auth_contracts(sandbox).await; - new_account(sandbox, "signer"); + let (id1, id2) = deploy_auth_contracts(sandbox).await; + sandbox .new_assert_cmd("contract") .arg("invoke") @@ -75,12 +77,13 @@ async fn root_auth_with_authorized_subcall() { } #[tokio::test] -async fn non_root_auth_subcall_fails() { +async fn non_root_auth_with_authorized_subcall() { let sandbox = &TestEnv::new(); - let (id1, id2) = deploy_auth_contracts(sandbox).await; - new_account(sandbox, "signer"); + let (id1, id2) = deploy_auth_contracts(sandbox).await; + + // with non-source signer - expect failure sandbox .new_assert_cmd("contract") .arg("invoke") @@ -95,13 +98,49 @@ async fn non_root_auth_subcall_fails() { .assert() .failure() .stderr(predicates::str::contains("Auth, InvalidAction")); + + // with source signer - expect failure + // TODO: this should pass once CLI supports non-root auth + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id1) + .arg("--") + .arg("no-auth-sub-auth") + .arg("--addr=test") + .arg("--val=hello") + .arg(&format!("--subcall={id2}")) + .assert() + .failure() + .stderr(predicates::str::contains("Auth, InvalidAction")); } #[tokio::test] -async fn partial_auth_source_account_fails() { +async fn partial_auth_with_authorized_subcall() { let sandbox = &TestEnv::new(); + new_account(sandbox, "signer"); + let (id1, id2) = deploy_auth_contracts(sandbox).await; + // with non-source signer - expect failure + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id1) + .arg("--") + .arg("partial_auth_sub_auth") + .arg("--addr=signer") + .arg("--val=hello") + .arg(&format!("--subcall={id2}")) + .assert() + .failure() + .stderr(predicates::str::contains("Signing authorization entries that could be submitted outside the context of the transaction is not supported in the CLI")); + + // with source signer - expect success sandbox .new_assert_cmd("contract") .arg("invoke") @@ -113,6 +152,17 @@ async fn partial_auth_source_account_fails() { .arg("--addr=test") .arg("--val=hello") .arg(&format!("--subcall={id2}")) + .assert() + .success() + .stdout("\"hello\"\n"); +} + +#[tokio::test] +async fn constructor_auth_with_non_source_signer() { + let sandbox = &TestEnv::new(); + new_account(sandbox, "signer"); + + constructor_cmd(sandbox, "signer") .assert() .failure() .stderr(predicates::str::contains("Auth, InvalidAction")); diff --git a/cmd/soroban-cli/src/auth.rs b/cmd/soroban-cli/src/auth.rs deleted file mode 100644 index d6c0ad64f0..0000000000 --- a/cmd/soroban-cli/src/auth.rs +++ /dev/null @@ -1,505 +0,0 @@ -use crate::{ - log::{auth, format_auth_entry}, - print, signer, - xdr::{ - AccountId, HostFunction, InvokeContractArgs, InvokeHostFunctionOp, MuxedAccount, Operation, - OperationBody, PublicKey, ScAddress, SorobanAuthorizationEntry, SorobanAuthorizedFunction, - SorobanCredentials, Transaction, Uint256, - }, -}; - -/// Check transactions for any malformed or non-strict authorization entries. These entries, if signed, -/// could be submitted outside of the context of the transaction's contract invocation. Note that -/// this function does not protect against interacting with a malicious contract, and should not -/// be relied on as protection against potentially malicious transactions. -/// -/// Enforces two checks: -/// -/// 1. **Source account uses SourceAccount credentials** -/// The source account's auth should use `SourceAccount` credentials. If it -/// appears as `Address`, the simulation's auth recording is not correct. -/// -/// 2. **Auth entry root invocation matches the tx operation** -/// Auth entries with `Address` credentials whose root invocation doesn't match the tx's -/// InvokeHostFunction could be used outside of the context of the transaction. -/// This does impact contracts that use `require_auth_for_args` as there is no way -/// to verify the authorization entry can't be recreated with different function arguments. -/// -/// This function also logs all auth entries that are caught by those two flags. -/// -/// # Errors -/// * If the source account credential is used directly instead of as a `SourceAccount` credential. -/// * If non-root auth is detected -pub fn check_auth(tx: &Transaction, quiet: bool) -> Result<(), signer::Error> { - let print = print::Print::new(quiet); - let source_bytes = source_account_bytes(tx); - // only need to check auth if op is `InvokeHostFunction` - let Some(invoke_host_op) = get_op(tx) else { - return Ok(()); - }; - - let mut non_strict_entries: Vec<&SorobanAuthorizationEntry> = Vec::new(); - for entry in invoke_host_op.auths() { - let SorobanCredentials::Address(ref creds) = entry.credentials else { - // SourceAccount credential entries are not signed explicitly - // so there is no risk of them being used outside the context of the transaction. - continue; - }; - - // Check if source account appears as Address credential - if let Some(auth_addr) = auth_address_bytes(&creds.address) { - if source_bytes == auth_addr { - print.warnln("Source account detected with Address credentials. This requires an extra signature and is not expected."); - print.warnln(format_auth_entry(entry)); - return Err(signer::Error::InvalidAuthEntry); - } - } - - // Check if the auth entry is strict. That is, it cannot be submitted outside the - // context of the transaction's host function. This check is overly strict as it doesn't - // allow for the usage of `require_auth_for_args`. - let is_strict = match &invoke_host_op.host_function { - // For `InvokeContract`, validate the root invocation matches the host function arguments. - HostFunction::InvokeContract(op) => match &entry.root_invocation.function { - SorobanAuthorizedFunction::ContractFn(auth_args) => auth_args == op, - _ => false, - }, - // For `CreateContract` and `CreateContractV2`, the root invocation should - // match the host function arguments. - HostFunction::CreateContract(op) => match &entry.root_invocation.function { - SorobanAuthorizedFunction::CreateContractHostFn(auth_args) => auth_args == op, - _ => false, - }, - HostFunction::CreateContractV2(op) => match &entry.root_invocation.function { - SorobanAuthorizedFunction::CreateContractV2HostFn(auth_args) => auth_args == op, - _ => false, - }, - // auth entries shouldn't exist for other host functions - HostFunction::UploadContractWasm(_) => { - print.warnln(format!( - "Auth entry not expected for the host function {}", - invoke_host_op.host_function.name() - )); - print.warnln(auth::format_auth_entry(entry)); - return Err(signer::Error::InvalidAuthEntry); - } - }; - - if !is_strict { - non_strict_entries.push(entry); - } - } - - if non_strict_entries.is_empty() { - Ok(()) - } else { - print.warnln( - "Authorization entries detected that could be submitted outside the context of this transaction:", - ); - for entry in non_strict_entries { - print.println(format_auth_entry(entry)); - } - Err(signer::Error::OutOfContextAuthEntry) - } -} - -/// Extract the Ed25519 public key bytes from a transaction's source account. -fn source_account_bytes(tx: &Transaction) -> [u8; 32] { - match &tx.source_account { - MuxedAccount::Ed25519(Uint256(bytes)) => *bytes, - MuxedAccount::MuxedEd25519(muxed) => muxed.ed25519.0, - } -} - -/// Extract the Ed25519 public key bytes from an auth entry's credential address. -fn auth_address_bytes(address: &ScAddress) -> Option<[u8; 32]> { - match address { - ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(bytes)))) => { - Some(*bytes) - } - _ => None, - } -} - -/// Extract the host function from a transaction, if it's an InvokeHostFunction operation. -fn get_op(tx: &Transaction) -> Option<&InvokeHostFunctionOp> { - let [Operation { - body: OperationBody::InvokeHostFunction(invoke_host_function_op), - .. - }] = tx.operations.as_slice() - else { - return None; - }; - Some(invoke_host_function_op) -} - -/// Extract the function name from a root invocation, if it's a contract function call. -pub fn invocation_function_name(auth: &SorobanAuthorizationEntry) -> Option { - match &auth.root_invocation.function { - SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { function_name, .. }) => { - std::str::from_utf8(function_name.as_ref()) - .ok() - .map(String::from) - } - _ => None, - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::xdr::{ - BytesM, ContractExecutable, ContractIdPreimage, ContractIdPreimageFromAddress, - CreateContractArgsV2, Hash, InvokeContractArgs, InvokeHostFunctionOp, Memo, Preconditions, - ScSymbol, ScVal, SequenceNumber, SorobanAddressCredentials, SorobanAuthorizedFunction, - SorobanAuthorizedInvocation, TransactionExt, VecM, - }; - use stellar_strkey::ed25519; - - const SOURCE_ACCOUNT: &str = "GBZXN7PIRZGNMHGA7MUUUF4GWPY5AYPV6LY4UV2GL6VJGIQRXFDNMADI"; - const OTHER_ACCOUNT: &str = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF"; - - fn source_account_bytes() -> [u8; 32] { - ed25519::PublicKey::from_string(SOURCE_ACCOUNT).unwrap().0 - } - - fn other_account_bytes() -> [u8; 32] { - ed25519::PublicKey::from_string(OTHER_ACCOUNT).unwrap().0 - } - - fn test_transaction( - host_fn: &HostFunction, - auth_entries: &[SorobanAuthorizationEntry], - ) -> Transaction { - Transaction { - source_account: MuxedAccount::Ed25519(Uint256(source_account_bytes())), - fee: 100, - seq_num: SequenceNumber(1), - cond: Preconditions::None, - memo: Memo::None, - operations: vec![Operation { - source_account: None, - body: OperationBody::InvokeHostFunction(InvokeHostFunctionOp { - host_function: host_fn.clone(), - auth: auth_entries.to_vec().try_into().unwrap(), - }), - }] - .try_into() - .unwrap(), - ext: TransactionExt::V0, - } - } - - fn make_host_fn_invoke_contract( - contract_addr: [u8; 32], - fn_name: &str, - args: &[ScVal], - ) -> HostFunction { - HostFunction::InvokeContract(InvokeContractArgs { - contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash( - contract_addr, - ))), - function_name: ScSymbol(fn_name.try_into().unwrap()), - args: args.try_into().unwrap(), - }) - } - - fn make_host_fn_create_contract(wasm_hash: [u8; 32], args: &[ScVal]) -> HostFunction { - HostFunction::CreateContractV2(CreateContractArgsV2 { - contract_id_preimage: ContractIdPreimage::Address(ContractIdPreimageFromAddress { - address: ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256( - source_account_bytes(), - )))), - salt: Uint256([0u8; 32]), - }), - executable: ContractExecutable::Wasm(wasm_hash.into()), - constructor_args: args.try_into().unwrap(), - }) - } - - fn make_auth_entry( - address_bytes: [u8; 32], - invocation: &SorobanAuthorizedInvocation, - ) -> SorobanAuthorizationEntry { - SorobanAuthorizationEntry { - credentials: SorobanCredentials::Address(SorobanAddressCredentials { - address: ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256( - address_bytes, - )))), - nonce: 0, - signature_expiration_ledger: 0, - signature: ScVal::Void, - }), - root_invocation: invocation.clone(), - } - } - - fn make_source_account_auth_entry( - invocation: &SorobanAuthorizedInvocation, - ) -> SorobanAuthorizationEntry { - SorobanAuthorizationEntry { - credentials: SorobanCredentials::SourceAccount, - root_invocation: invocation.clone(), - } - } - - fn make_auth_invocation_contract( - contract_addr: [u8; 32], - fn_name: &str, - args: &[ScVal], - ) -> SorobanAuthorizedInvocation { - SorobanAuthorizedInvocation { - function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { - contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash( - contract_addr, - ))), - function_name: ScSymbol(fn_name.try_into().unwrap()), - args: args.to_vec().try_into().unwrap(), - }), - sub_invocations: VecM::default(), - } - } - - fn make_auth_invocation_create_contract( - wasm_hash: [u8; 32], - args: &[ScVal], - ) -> SorobanAuthorizedInvocation { - SorobanAuthorizedInvocation { - function: SorobanAuthorizedFunction::CreateContractV2HostFn(CreateContractArgsV2 { - contract_id_preimage: ContractIdPreimage::Address(ContractIdPreimageFromAddress { - address: ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519( - Uint256(source_account_bytes()), - ))), - salt: Uint256([0u8; 32]), - }), - executable: ContractExecutable::Wasm(wasm_hash.into()), - constructor_args: args.try_into().unwrap(), - }), - sub_invocations: VecM::default(), - } - } - - #[test] - fn test_matching_root_invocation_passes() { - let contract = [1u8; 32]; - let other_contract = [99u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - let mut invocation = make_auth_invocation_contract(contract, "hello", args); - let sub_invocation = make_auth_invocation_contract(other_contract, "other", &[]); - invocation.sub_invocations = [sub_invocation].try_into().unwrap(); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_matching_root_invocation_with_subinvocations_passes() { - let contract = [1u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - let invocation = make_auth_invocation_contract(contract, "hello", args); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_source_account_as_address_credential_is_rejected() { - let contract = [1u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - let invocation = make_auth_invocation_contract(contract, "hello", args); - let auth = make_auth_entry(source_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } - - #[test] - fn test_source_account_credentials_passes() { - let contract = [1u8; 32]; - let other_contract = [99u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - let invocation = make_auth_invocation_contract(other_contract, "other", &[]); - let auth = make_source_account_auth_entry(&invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_non_root_auth_is_rejected() { - let contract = [1u8; 32]; - let other_contract = [99u8; 32]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", &[]); - let invocation = make_auth_invocation_contract(other_contract, "hello", &[]); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } - - #[test] - fn test_different_function_same_contract_is_rejected() { - let contract = [1u8; 32]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", &[]); - let invocation = make_auth_invocation_contract(contract, "transfer", &[]); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } - - #[test] - fn test_different_args_is_rejected() { - let contract = [1u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - let wrong_args = &[ScVal::U32(43), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - let invocation = make_auth_invocation_contract(contract, "hello", wrong_args); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } - - #[test] - fn test_multiple_valid_passes() { - let contract = [1u8; 32]; - let other_contract = [99u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - - // Root-matching auth — safe - let root_invocation = make_auth_invocation_contract(contract, "hello", args); - let safe_auth = make_auth_entry(other_account_bytes(), &root_invocation); - - // SourceAccount auth for different contract — safe - let other_invocation = make_auth_invocation_contract(other_contract, "anything", &[]); - let source_auth = make_source_account_auth_entry(&other_invocation); - - // Root auth for different contract - let other_invocation = make_auth_invocation_contract(contract, "hello", args); - let other_auth = make_auth_entry(other_account_bytes(), &other_invocation); - - let tx = test_transaction(&host_fn, &[safe_auth, source_auth, other_auth]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_rejects_if_any_fail_check() { - let contract = [1u8; 32]; - let other_contract = [99u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_invoke_contract(contract, "hello", args); - - // Root-matching auth — safe - let root_invocation = make_auth_invocation_contract(contract, "hello", args); - let safe_auth = make_auth_entry(other_account_bytes(), &root_invocation); - - // SourceAccount auth for different contract — safe - let other_invocation = make_auth_invocation_contract(other_contract, "anything", &[]); - let source_auth = make_source_account_auth_entry(&other_invocation); - - // Non-Root auth for different contract - let other_invocation = make_auth_invocation_contract(other_contract, "hello", args); - let other_auth = make_auth_entry(other_account_bytes(), &other_invocation); - - let tx = test_transaction(&host_fn, &[safe_auth, source_auth, other_auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } - - #[test] - fn test_upload_wasm_host_function_passes() { - let wasm_hash: BytesM = [42u8; 32].try_into().unwrap(); - - let host_fn = HostFunction::UploadContractWasm(wasm_hash); - let tx = test_transaction(&host_fn, &[]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_upload_wasm_host_function_with_auth_is_rejected() { - let contract = [1u8; 32]; - let wasm_hash: BytesM = [42u8; 32].try_into().unwrap(); - - let host_fn = HostFunction::UploadContractWasm(wasm_hash); - let invocation = make_auth_invocation_contract(contract, "hello", &[]); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } - - #[test] - fn test_deploy_no_constructor_passes() { - let wasm_hash = [42u8; 32]; - - let host_fn = make_host_fn_create_contract(wasm_hash, &[]); - let tx = test_transaction(&host_fn, &[]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_deploy_constructor_passes() { - let contract = [1u8; 32]; - let wasm_hash = [42u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_create_contract(wasm_hash, args); - let mut invocation = make_auth_invocation_create_contract(wasm_hash, args); - let sub_invocation = make_auth_invocation_contract(contract, "__constructor", args); - invocation.sub_invocations = [sub_invocation].try_into().unwrap(); - let auth = make_auth_entry(other_account_bytes(), &invocation); - let tx = test_transaction(&host_fn, &[auth]); - - let result = check_auth(&tx, true); - assert!(result.is_ok()); - } - - #[test] - fn test_deploy_constructor_with_non_source_auth() { - let contract = [1u8; 32]; - let wasm_hash = [42u8; 32]; - let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; - - let host_fn = make_host_fn_create_contract(wasm_hash, args); - let invocation = make_auth_invocation_create_contract(wasm_hash, args); - let auth = make_auth_entry(source_account_bytes(), &invocation); - let other_invocation = make_auth_invocation_contract(contract, "__constructor", args); - let other_auth = make_auth_entry(other_account_bytes(), &other_invocation); - let tx = test_transaction(&host_fn, &[auth, other_auth]); - - let result = check_auth(&tx, true); - assert!(result.is_err()); - } -} diff --git a/cmd/soroban-cli/src/lib.rs b/cmd/soroban-cli/src/lib.rs index 89778c1614..ae411d63a6 100644 --- a/cmd/soroban-cli/src/lib.rs +++ b/cmd/soroban-cli/src/lib.rs @@ -12,7 +12,6 @@ mod cli; pub use cli::main; pub mod assembled; -pub mod auth; pub mod color; pub mod commands; pub mod config; diff --git a/cmd/soroban-cli/src/log/auth.rs b/cmd/soroban-cli/src/log/auth.rs index 910335691e..315d9a5d8b 100644 --- a/cmd/soroban-cli/src/log/auth.rs +++ b/cmd/soroban-cli/src/log/auth.rs @@ -2,19 +2,12 @@ use std::fmt::Write; use crate::xdr::{ AccountId, InvokeContractArgs, PublicKey, ScAddress, SorobanAuthorizationEntry, - SorobanAuthorizedFunction, SorobanAuthorizedInvocation, SorobanCredentials, Uint256, VecM, + SorobanAuthorizedFunction, SorobanAuthorizedInvocation, SorobanCredentials, Uint256, }; -/// Legacy debug logging function (preserved for backward compatibility via `pub use auth::*`) -pub fn auth(auth: &[VecM]) { - if !auth.is_empty() { - tracing::debug!("{auth:#?}"); - } -} - /// Format a single auth entry for display. pub fn format_auth_entry(entry: &SorobanAuthorizationEntry) -> String { - let mut result = format!(" Auth Entry:\n"); + let mut result = String::from(" Auth Entry:\n"); match &entry.credentials { SorobanCredentials::Address(creds) => { @@ -25,19 +18,22 @@ pub fn format_auth_entry(entry: &SorobanAuthorizationEntry) -> String { } } - format_invocation(&entry.root_invocation, 2, 0, &mut result); + format_invocation(&entry.root_invocation, 2, "Invocation:", &mut result); result } -/// Recursively format a `SorobanAuthorizedInvocation` tree. +/// Recursively format a `SorobanAuthorizedInvocation` tree. `label` is the +/// header line printed for this node — `"Invocation:"` for the root and +/// `"Sub-invocation #N:"` for each child. fn format_invocation( invocation: &SorobanAuthorizedInvocation, indent: usize, - index: usize, + label: &str, result: &mut String, ) { let prefix = " ".repeat(indent); + let _ = writeln!(result, "{prefix}{label}"); match &invocation.function { SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { @@ -46,7 +42,6 @@ fn format_invocation( args, }) => { let fn_name = std::str::from_utf8(function_name.as_ref()).unwrap_or(""); - let _ = writeln!(result, "{prefix}Function #{index}:"); let _ = writeln!( result, "{prefix} Contract: {}", @@ -67,13 +62,13 @@ fn format_invocation( } SorobanAuthorizedFunction::CreateContractHostFn(_) | SorobanAuthorizedFunction::CreateContractV2HostFn(_) => { - let _ = writeln!(result, "{prefix}Function #{index}:"); let _ = writeln!(result, "{prefix} CreateContract"); } } for (i, sub) in invocation.sub_invocations.iter().enumerate() { - format_invocation(sub, indent + 1, i, result); + let sub_label = format!("Sub-invocation #{i}:"); + format_invocation(sub, indent + 1, &sub_label, result); } } diff --git a/cmd/soroban-cli/src/signer/mod.rs b/cmd/soroban-cli/src/signer/mod.rs index ae34c94028..529edc9350 100644 --- a/cmd/soroban-cli/src/signer/mod.rs +++ b/cmd/soroban-cli/src/signer/mod.rs @@ -1,11 +1,12 @@ use crate::{ + log::format_auth_entry, utils::fee_bump_transaction_hash, xdr::{ self, AccountId, DecoratedSignature, FeeBumpTransactionEnvelope, Hash, HashIdPreimage, - HashIdPreimageSorobanAuthorization, Limits, Operation, OperationBody, PublicKey, ScAddress, - ScMap, ScSymbol, ScVal, Signature, SignatureHint, SorobanAddressCredentials, - SorobanAuthorizationEntry, SorobanCredentials, Transaction, TransactionEnvelope, - TransactionV1Envelope, Uint256, VecM, WriteXdr, + HashIdPreimageSorobanAuthorization, Limits, MuxedAccount, Operation, OperationBody, + PublicKey, ScAddress, ScMap, ScSymbol, ScVal, Signature, SignatureHint, + SorobanAddressCredentials, SorobanAuthorizationEntry, SorobanCredentials, Transaction, + TransactionEnvelope, TransactionV1Envelope, Uint256, VecM, WriteXdr, }, }; use ed25519_dalek::{ed25519::signature::Signer as _, Signature as Ed25519Signature}; @@ -14,6 +15,7 @@ use sha2::{Digest, Sha256}; use crate::{config::network::Network, print::Print, utils::transaction_hash}; pub mod ledger; +pub mod validation; #[cfg(feature = "additional-libs")] mod keyring; @@ -29,10 +31,13 @@ pub enum Error { MissingSignerForAddress { address: String }, #[error(transparent)] TryFromSlice(#[from] std::array::TryFromSliceError), - #[error("Signing authorization entries that could be submitted outside the context of the transaction is not supported in the CLI")] - OutOfContextAuthEntry, - #[error("Invalid Soroban authorization entry")] - InvalidAuthEntry, + #[error("Signing authorization entries that could be submitted outside the context of the transaction is not supported in the CLI:\n{auth_entry_str}")] + NotStrictAuthEntry { auth_entry_str: String }, + #[error("Invalid Soroban authorization entry - {reason}:\n{auth_entry_str}")] + InvalidAuthEntry { + reason: String, + auth_entry_str: String, + }, #[error(transparent)] Xdr(#[from] xdr::Error), #[error("Transaction envelope type not supported")] @@ -74,6 +79,7 @@ pub fn sign_soroban_authorizations( }; let network_id = Hash(Sha256::digest(network_passphrase.as_bytes()).into()); + let source_bytes = muxed_account_bytes(&raw.source_account); let mut auths_modified = false; let mut signed_auths = Vec::with_capacity(body.auth.len()); @@ -90,9 +96,25 @@ pub fn sign_soroban_authorizations( }; let SorobanAddressCredentials { ref address, .. } = credentials; + // Before we attempt to sign, validate the auth entry is strict + match validation::classify_auth_invocation(&body.host_function, &auth.root_invocation) { + validation::AuthStyle::Strict => {} + validation::AuthStyle::NonStrict => { + return Err(Error::NotStrictAuthEntry { + auth_entry_str: format_auth_entry(&auth), + }); + } + validation::AuthStyle::Invalid => { + return Err(Error::InvalidAuthEntry { + reason: "authorization entry is not expected for the transaction".to_string(), + auth_entry_str: format_auth_entry(&auth), + }); + } + } + // See if we have a signer for this authorizationEntry // If not, then we Error - let needle: &[u8; 32] = match address { + let auth_address_bytes: &[u8; 32] = match address { ScAddress::MuxedAccount(_) => todo!("muxed accounts are not supported"), ScAddress::ClaimableBalance(_) => todo!("claimable balance not supported"), ScAddress::LiquidityPool(_) => todo!("liquidity pool not supported"), @@ -107,9 +129,17 @@ pub fn sign_soroban_authorizations( } }; + // Auth entries should not request a signature from the tx source account via the `Address` credential type + if auth_address_bytes == source_bytes { + return Err(Error::InvalidAuthEntry { + reason: "transaction source account is used as credentials".to_string(), + auth_entry_str: format_auth_entry(&auth), + }); + } + let mut signer: Option<&Signer> = None; for s in signers { - if needle == &s.get_public_key()?.0 { + if auth_address_bytes == &s.get_public_key()?.0 { signer = Some(s); } } @@ -128,7 +158,7 @@ pub fn sign_soroban_authorizations( None => { return Err(Error::MissingSignerForAddress { address: stellar_strkey::Strkey::PublicKeyEd25519( - stellar_strkey::ed25519::PublicKey(*needle), + stellar_strkey::ed25519::PublicKey(*auth_address_bytes), ) .to_string(), }); @@ -379,3 +409,230 @@ impl SecureStoreEntry { Ok(sig) } } + +/// Extract the Ed25519 public key bytes from a MuxedAccount +fn muxed_account_bytes(source: &MuxedAccount) -> &[u8; 32] { + match source { + MuxedAccount::Ed25519(Uint256(bytes)) => bytes, + MuxedAccount::MuxedEd25519(muxed) => &muxed.ed25519.0, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::xdr::{ + BytesM, HostFunction, InvokeContractArgs, InvokeHostFunctionOp, Memo, Preconditions, + SequenceNumber, SorobanAuthorizedFunction, SorobanAuthorizedInvocation, TransactionExt, + }; + + const NETWORK: &str = "Test SDF Network ; September 2015"; + const EXPIRATION_LEDGER: u32 = 100; + + fn local_signer(seed: [u8; 32]) -> Signer { + Signer { + kind: SignerKind::Local(LocalKey { + key: ed25519_dalek::SigningKey::from_bytes(&seed), + }), + print: Print::new(true), + } + } + + fn signer_pubkey(signer: &Signer) -> [u8; 32] { + signer.get_public_key().unwrap().0 + } + + fn ed25519_address(bytes: [u8; 32]) -> ScAddress { + ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(bytes)))) + } + + fn invoke_args(contract: [u8; 32], fn_name: &str) -> InvokeContractArgs { + InvokeContractArgs { + contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash(contract))), + function_name: ScSymbol(fn_name.try_into().unwrap()), + args: VecM::default(), + } + } + + fn invocation(contract: [u8; 32], fn_name: &str) -> SorobanAuthorizedInvocation { + SorobanAuthorizedInvocation { + function: SorobanAuthorizedFunction::ContractFn(invoke_args(contract, fn_name)), + sub_invocations: VecM::default(), + } + } + + fn address_auth( + address: ScAddress, + invocation: SorobanAuthorizedInvocation, + ) -> SorobanAuthorizationEntry { + SorobanAuthorizationEntry { + credentials: SorobanCredentials::Address(SorobanAddressCredentials { + address, + nonce: 0, + signature_expiration_ledger: 0, + signature: ScVal::Void, + }), + root_invocation: invocation, + } + } + + fn build_tx( + source: MuxedAccount, + host_function: HostFunction, + auth: Vec, + ) -> Transaction { + Transaction { + source_account: source, + fee: 100, + seq_num: SequenceNumber(1), + cond: Preconditions::None, + memo: Memo::None, + operations: vec![Operation { + source_account: None, + body: OperationBody::InvokeHostFunction(InvokeHostFunctionOp { + host_function, + auth: auth.try_into().unwrap(), + }), + }] + .try_into() + .unwrap(), + ext: TransactionExt::V0, + } + } + + /// Pull the embedded public_key bytes out of a signed Address-cred entry. + fn extract_signed_pubkey(creds: &SorobanAddressCredentials) -> [u8; 32] { + let ScVal::Vec(Some(outer)) = &creds.signature else { + panic!("expected ScVal::Vec signature"); + }; + let Some(ScVal::Map(Some(map))) = outer.first() else { + panic!("expected ScVal::Map inside signature vec"); + }; + map.iter() + .find_map(|e| match (&e.key, &e.val) { + (ScVal::Symbol(s), ScVal::Bytes(b)) if s.0.as_slice() == b"public_key" => { + Some(b.as_slice().try_into().unwrap()) + } + _ => None, + }) + .expect("public_key entry") + } + + #[test] + fn test_signs_address_auth_entry_with_matching_signer() { + let signer = local_signer([1u8; 32]); + let signer_unused = local_signer([2u8; 32]); + let signer_pk = signer_pubkey(&signer); + let source = MuxedAccount::Ed25519(Uint256([9u8; 32])); + let contract = [42u8; 32]; + + let entry = address_auth(ed25519_address(signer_pk), invocation(contract, "hello")); + let host_fn = HostFunction::InvokeContract(invoke_args(contract, "hello")); + let tx = build_tx(source, host_fn, vec![entry]); + + let signed_auth_tx = + sign_soroban_authorizations(&tx, &[signer_unused, signer], EXPIRATION_LEDGER, NETWORK) + .unwrap() + .expect("signing modifies the transaction"); + + let OperationBody::InvokeHostFunction(body) = &signed_auth_tx.operations[0].body else { + panic!("expected InvokeHostFunction"); + }; + let SorobanCredentials::Address(creds) = &body.auth[0].credentials else { + panic!("expected Address credentials"); + }; + assert!( + !matches!(creds.signature, ScVal::Void), + "signature should be filled in" + ); + assert_eq!(creds.signature_expiration_ledger, EXPIRATION_LEDGER); + assert_eq!( + extract_signed_pubkey(creds), + signer_pk, + "embedded public_key should match the signer" + ); + } + + #[test] + fn test_non_strict_auth_returns_error() { + let signer = local_signer([1u8; 32]); + let signer_pk = signer_pubkey(&signer); + let source = MuxedAccount::Ed25519(Uint256([9u8; 32])); + let contract = [42u8; 32]; + let other_contract = [99u8; 32]; + + let entry = address_auth( + ed25519_address(signer_pk), + invocation(other_contract, "hello"), + ); + let host_fn = HostFunction::InvokeContract(invoke_args(contract, "hello")); + let tx = build_tx(source, host_fn, vec![entry]); + + let result = sign_soroban_authorizations(&tx, &[signer], EXPIRATION_LEDGER, NETWORK); + assert!(matches!(result, Err(Error::NotStrictAuthEntry { .. }))); + } + + #[test] + fn test_multiple_entries_with_non_strict_returns_error() { + let signer = local_signer([1u8; 32]); + let signer_pk = signer_pubkey(&signer); + let source = MuxedAccount::Ed25519(Uint256([9u8; 32])); + let contract = [42u8; 32]; + let other_contract = [99u8; 32]; + + let entry = address_auth(ed25519_address(signer_pk), invocation(contract, "hello")); + let entry_non_strict = address_auth( + ed25519_address(signer_pk), + invocation(other_contract, "hello"), + ); + let host_fn = HostFunction::InvokeContract(invoke_args(contract, "hello")); + let tx = build_tx(source, host_fn, vec![entry, entry_non_strict]); + + let result = sign_soroban_authorizations(&tx, &[signer], EXPIRATION_LEDGER, NETWORK); + assert!(matches!(result, Err(Error::NotStrictAuthEntry { .. }))); + } + + #[test] + fn test_upload_wasm_with_auth_returns_invalid() { + let signer = local_signer([1u8; 32]); + let signer_pk = signer_pubkey(&signer); + let source = MuxedAccount::Ed25519(Uint256([9u8; 32])); + let wasm: BytesM = [0u8; 32].try_into().unwrap(); + + let entry = address_auth(ed25519_address(signer_pk), invocation([42u8; 32], "hello")); + let host_fn = HostFunction::UploadContractWasm(wasm); + let tx = build_tx(source, host_fn, vec![entry]); + + let result = sign_soroban_authorizations(&tx, &[signer], EXPIRATION_LEDGER, NETWORK); + assert!(matches!(result, Err(Error::InvalidAuthEntry { .. }))); + } + + #[test] + fn test_source_account_as_address_returns_invalid() { + let signer = local_signer([1u8; 32]); + let signer_pk = signer_pubkey(&signer); + let source = MuxedAccount::Ed25519(Uint256(signer_pk)); + let contract = [42u8; 32]; + + let entry = address_auth(ed25519_address(signer_pk), invocation(contract, "hello")); + let host_fn = HostFunction::InvokeContract(invoke_args(contract, "hello")); + let tx = build_tx(source, host_fn, vec![entry]); + + let result = sign_soroban_authorizations(&tx, &[signer], EXPIRATION_LEDGER, NETWORK); + assert!(matches!(result, Err(Error::InvalidAuthEntry { .. }))); + } + + #[test] + fn test_missing_signer_returns_error() { + let source = MuxedAccount::Ed25519(Uint256([9u8; 32])); + let contract = [42u8; 32]; + let unknown = [77u8; 32]; + + let entry = address_auth(ed25519_address(unknown), invocation(contract, "hello")); + let host_fn = HostFunction::InvokeContract(invoke_args(contract, "hello")); + let tx = build_tx(source, host_fn, vec![entry]); + + let result = sign_soroban_authorizations(&tx, &[], EXPIRATION_LEDGER, NETWORK); + assert!(matches!(result, Err(Error::MissingSignerForAddress { .. }))); + } +} diff --git a/cmd/soroban-cli/src/signer/validation.rs b/cmd/soroban-cli/src/signer/validation.rs new file mode 100644 index 0000000000..8cdc178686 --- /dev/null +++ b/cmd/soroban-cli/src/signer/validation.rs @@ -0,0 +1,222 @@ +use crate::xdr::{HostFunction, SorobanAuthorizedFunction, SorobanAuthorizedInvocation}; + +/// Classification of an `Address`-credential auth entry's relationship to the +/// transaction's host function. +/// +/// `SourceAccount` credential entries are out of scope here — they are signed +/// implicitly via the transaction envelope and never reach this classifier. +#[derive(Debug, PartialEq, Eq)] +pub enum AuthStyle { + /// `root_invocation` matches the host function exactly. Safe to sign: + /// the entry is bound to this transaction host function and cannot be replayed. + Strict, + /// `root_invocation` does not match the host function exactly. Signing this + /// could produce a portable authorization that could be submitted + /// outside the context of this transaction. + NonStrict, + /// `root_invocation` is not expected for the host function + Invalid, +} + +/// Classify an auth invocation against the transaction's host function. +/// +/// ### Arguments +/// * `source_host_fn`- The transaction's host function +/// * `auth_invocation` - The auth entry's root invocation +pub fn classify_auth_invocation( + source_host_fn: &HostFunction, + auth_invocation: &SorobanAuthorizedInvocation, +) -> AuthStyle { + // No auth entries are valid for `UploadContractWasm`. + if matches!(source_host_fn, HostFunction::UploadContractWasm(_)) { + return AuthStyle::Invalid; + } + + // Check if the auth entry's root invocation matches the host function exactly. + // This is different than just a `root_auth` check, as contracts that authorize with + // `require_auth_for_args` at the root are not considered strict auth. This tradeoff is + // made to ensure that even a tampered auth entry can be flagged as non-strict. + let is_strict = match (source_host_fn, &auth_invocation.function) { + (HostFunction::InvokeContract(op), SorobanAuthorizedFunction::ContractFn(args)) => { + args == op + } + ( + HostFunction::CreateContract(op), + SorobanAuthorizedFunction::CreateContractHostFn(args), + ) => args == op, + ( + HostFunction::CreateContractV2(op), + SorobanAuthorizedFunction::CreateContractV2HostFn(args), + ) => args == op, + _ => false, + }; + + if is_strict { + AuthStyle::Strict + } else { + AuthStyle::NonStrict + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::xdr::{ + AccountId, BytesM, ContractExecutable, ContractIdPreimage, ContractIdPreimageFromAddress, + CreateContractArgsV2, Hash, InvokeContractArgs, PublicKey, ScAddress, ScSymbol, ScVal, + Uint256, VecM, + }; + use stellar_strkey::ed25519; + + const SOURCE_ACCOUNT: &str = "GBZXN7PIRZGNMHGA7MUUUF4GWPY5AYPV6LY4UV2GL6VJGIQRXFDNMADI"; + + fn source_bytes() -> [u8; 32] { + ed25519::PublicKey::from_string(SOURCE_ACCOUNT).unwrap().0 + } + + fn ed25519_address(bytes: [u8; 32]) -> ScAddress { + ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(bytes)))) + } + + fn host_fn_invoke(contract: [u8; 32], fn_name: &str, args: &[ScVal]) -> HostFunction { + HostFunction::InvokeContract(InvokeContractArgs { + contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash(contract))), + function_name: ScSymbol(fn_name.try_into().unwrap()), + args: args.try_into().unwrap(), + }) + } + + fn host_fn_create(wasm_hash: [u8; 32], args: &[ScVal]) -> HostFunction { + HostFunction::CreateContractV2(CreateContractArgsV2 { + contract_id_preimage: ContractIdPreimage::Address(ContractIdPreimageFromAddress { + address: ed25519_address(source_bytes()), + salt: Uint256([0u8; 32]), + }), + executable: ContractExecutable::Wasm(wasm_hash.into()), + constructor_args: args.try_into().unwrap(), + }) + } + + fn invocation_contract( + contract: [u8; 32], + fn_name: &str, + args: &[ScVal], + ) -> SorobanAuthorizedInvocation { + SorobanAuthorizedInvocation { + function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs { + contract_address: ScAddress::Contract(stellar_xdr::curr::ContractId(Hash( + contract, + ))), + function_name: ScSymbol(fn_name.try_into().unwrap()), + args: args.to_vec().try_into().unwrap(), + }), + sub_invocations: VecM::default(), + } + } + + fn invocation_create(wasm_hash: [u8; 32], args: &[ScVal]) -> SorobanAuthorizedInvocation { + SorobanAuthorizedInvocation { + function: SorobanAuthorizedFunction::CreateContractV2HostFn(CreateContractArgsV2 { + contract_id_preimage: ContractIdPreimage::Address(ContractIdPreimageFromAddress { + address: ed25519_address(source_bytes()), + salt: Uint256([0u8; 32]), + }), + executable: ContractExecutable::Wasm(wasm_hash.into()), + constructor_args: args.try_into().unwrap(), + }), + sub_invocations: VecM::default(), + } + } + + #[test] + fn test_matching_root_invocation_is_strict() { + let contract = [1u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = host_fn_invoke(contract, "hello", args); + let invocation = invocation_contract(contract, "hello", args); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::Strict); + } + + #[test] + fn test_subinvocations_dont_affect_root_match() { + let contract = [1u8; 32]; + let other = [99u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = host_fn_invoke(contract, "hello", args); + let mut invocation = invocation_contract(contract, "hello", args); + invocation.sub_invocations = [invocation_contract(other, "other", &[])] + .try_into() + .unwrap(); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::Strict); + } + + #[test] + fn test_different_root_contract_is_non_strict() { + let contract = [1u8; 32]; + let other = [99u8; 32]; + + let host_fn = host_fn_invoke(contract, "hello", &[]); + let invocation = invocation_contract(other, "hello", &[]); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::NonStrict); + } + + #[test] + fn test_different_function_same_contract_is_non_strict() { + let contract = [1u8; 32]; + + let host_fn = host_fn_invoke(contract, "hello", &[]); + let invocation = invocation_contract(contract, "transfer", &[]); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::NonStrict); + } + + #[test] + fn test_different_args_is_non_strict() { + let contract = [1u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + let wrong = &[ScVal::U32(43), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = host_fn_invoke(contract, "hello", args); + let invocation = invocation_contract(contract, "hello", wrong); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::NonStrict); + } + + #[test] + fn test_upload_wasm_with_auth_entry_is_invalid() { + let contract = [1u8; 32]; + let wasm_hash: BytesM = [42u8; 32].try_into().unwrap(); + + let host_fn = HostFunction::UploadContractWasm(wasm_hash); + let invocation = invocation_contract(contract, "hello", &[]); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::Invalid); + } + + #[test] + fn test_matching_create_contract_root_is_strict() { + let contract = [1u8; 32]; + let wasm_hash = [42u8; 32]; + let args = &[ScVal::U32(42), ScVal::Symbol("hello".try_into().unwrap())]; + + let host_fn = host_fn_create(wasm_hash, args); + let mut invocation = invocation_create(wasm_hash, args); + invocation.sub_invocations = [invocation_contract(contract, "__constructor", args)] + .try_into() + .unwrap(); + + let style = classify_auth_invocation(&host_fn, &invocation); + assert_eq!(style, AuthStyle::Strict); + } +} diff --git a/cmd/soroban-cli/src/tx.rs b/cmd/soroban-cli/src/tx.rs index 7d3a543103..4aebfb4a7c 100644 --- a/cmd/soroban-cli/src/tx.rs +++ b/cmd/soroban-cli/src/tx.rs @@ -1,6 +1,5 @@ use crate::{ assembled::simulate_and_assemble_transaction, - auth::check_auth, commands::tx::fetch, config::{self, data, network}, print, resources, @@ -66,11 +65,8 @@ where data::write(sim_res.clone().into(), &network.rpc_uri()?)?; } - // Check for malformed, or non-strict auth entries that could be submitted - // outside the context of this transaction - check_auth(&txn, quiet).map_err(config::Error::from)?; - - // Sign all auth entries now that they've been validated/approved + // Sign all auth entries. Each entry is validated against the transaction's + // host function inside `sign_soroban_authorizations` before being signed. if let Some(tx) = config .sign_soroban_authorizations(&txn, auth_signers) .await?