Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions contracts/smart-account/src/auth/core/authorizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,23 @@ impl Authorizer {
{
let res = SmartAccountPluginClient::new(env, &plugin)
.try_on_auth(&env.current_contract_address(), auth_contexts);
// Plugin-result classification (Soroban 22 try_* semantics):
// Ok(Ok(_)) — plugin returned normally → continue
// Ok(Err(_)) — return-value ABI decode failure → SKIP (fail-open)
// Err(Ok(_)) — contracterror OR panic!/unwrap/unreachable → REJECT (fail-closed)
// Err(Err(_)) — host trap (archival, budget, missing code) → SKIP (fail-open)
//
// Note: any `panic!` from the plugin — including an unwrap that
// fires or an arithmetic overflow — lands in the Err(Ok(_))
// branch and blocks auth. Only environmental host traps
// (storage archival, budget exhaustion, stack overflow,
// contract-not-found) reach Err(Err(_)) and are silently
// skipped. Plugins that want to stay enforceable should
// manage their own TTL and avoid unbounded work.
match res {
// Plugin executed successfully
Ok(Ok(_)) => {}
// Plugin return value conversion failure (ABI mismatch)
// Plugin return value ABI decode failure
// Treat as technical failure: log and continue
Ok(Err(_)) => {
env.events().publish(
Expand All @@ -105,7 +118,8 @@ impl Authorizer {
},
);
}
// Plugin intentionally rejected (contracterror / panic_with_error!)
// Plugin intentionally rejected (contracterror) or panicked
// (panic!, panic_with_error!, unwrap, arithmetic overflow, etc.)
Err(Ok(_)) => {
env.events().publish(
(TOPIC_PLUGIN, &plugin, VERB_AUTH_FAILED),
Expand All @@ -116,8 +130,10 @@ impl Authorizer {
);
return Err(Error::PluginOnAuthFailed);
}
// Plugin had a technical failure (panic!, host trap, TTL expiry)
// Non-blocking: log and continue to next plugin
// Host-level failure: storage archival, budget exhaustion,
// contract not found, stack overflow. Non-blocking: log
// and continue to next plugin so an unreachable plugin
// cannot brick the account.
Err(Err(_)) => {
env.events().publish(
(TOPIC_PLUGIN, &plugin, VERB_AUTH_FAILED),
Expand Down
59 changes: 59 additions & 0 deletions contracts/smart-account/src/tests/plugin_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,62 @@ fn test_max_plugins_limit() {

assert_eq!(err, Error::MaxPluginsReached);
}

// -----------------------------------------------------------------------------
// Plugin that crashes via plain `panic!` — Soroban 22 classifies this as
// Err(Ok(_)), i.e. fail-closed. The authorizer comment previously grouped
// `panic!` with host traps in the fail-open branch, which was inaccurate.
// This test pins the actual behaviour so future refactors cannot regress it.
// -----------------------------------------------------------------------------

mod panicking_plugin {
use soroban_sdk::{auth::Context, contract, contractimpl, Address, Env, Vec};

#[contract]
pub struct PlainPanicPlugin;

#[contractimpl]
impl PlainPanicPlugin {
pub fn on_install(_env: &Env, _source: Address) {}
pub fn on_uninstall(_env: &Env, _source: Address) {}
#[allow(clippy::panic)]
pub fn on_auth(_env: &Env, _source: Address, _contexts: Vec<Context>) {
panic!("buggy plugin — this should block auth, not be skipped");
}
}
}

#[test]
fn test_plugin_plain_panic_is_fail_closed() {
let env = setup();
env.mock_all_auths();

let admin = Ed25519TestSigner::generate(SignerRole::Admin);
let smart_account_id = env.register(
SmartAccount,
(
vec![&env, admin.into_signer(&env)],
Vec::<Address>::new(&env),
),
);

let plugin_id = env.register(panicking_plugin::PlainPanicPlugin, ());
env.as_contract(&smart_account_id, || {
SmartAccount::install_plugin(&env, plugin_id.clone())
})
.unwrap();

let payload = BytesN::random(&env);
let (admin_key, admin_proof) = admin.sign(&env, &payload);
let auth_payloads = SignatureProofs(soroban_sdk::map![&env, (admin_key, admin_proof)]);

let result = env.try_invoke_contract_check_auth::<Error>(
&smart_account_id,
&payload,
auth_payloads.into_val(&env),
&vec![&env, get_token_auth_context(&env)],
);

// plain `panic!` → Err(Ok(_)) classification → PluginOnAuthFailed.
assert_eq!(result, Err(Ok(Error::PluginOnAuthFailed)));
}
Loading