From 1bd5535c46533c0f062d4bad541394f95699dff9 Mon Sep 17 00:00:00 2001 From: mootz12 Date: Fri, 3 Apr 2026 16:14:05 -0400 Subject: [PATCH 1/2] 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/2] 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) => {