[Bug]: No cancel_upgrade — a hostile admin's pending upgrade survives a successful guardian recovery and permanently blocks all future propose_upgrade calls

[ndii-dev]

Context

GlobeWallet::propose_upgrade / execute_upgrade, contracts/globe-wallet/src/lib.rs. Compare against the other two "propose then later act" subsystems in this same file: admin transfer has cancel_admin_transfer, guardian recovery has cancel_recovery. The upgrade subsystem has no equivalent.

Problem

$ grep -n "pub fn.*upgrade\|PendingUpgrade" contracts/globe-wallet/src/lib.rs
31:    PendingUpgrade,
326:    pub fn propose_upgrade(
334:        if env.storage().instance().has(&DataKey::PendingUpgrade) {
378:    pub fn execute_upgrade(
388:            .get(&DataKey::PendingUpgrade)
416:        env.storage().instance().remove(&DataKey::PendingUpgrade);

Only two functions ever touch PendingUpgrade: propose_upgrade (sets it, and refuses to overwrite an existing proposal — UpgradeAlreadyPending) and execute_upgrade (clears it, but only on success). There is no cancel_upgrade. execute_recovery — which explicitly documents that it cancels any in-flight admin transfer as part of firing — does not touch PendingUpgrade either.

Walk through the scenario guardian recovery exists for: an admin's key is compromised. Before the legitimate owner's guardians can act, the attacker calls propose_upgrade with a backdoored WASM hash and a short delay. Guardians successfully recover admin to a new, trusted address. The malicious PendingUpgrade entry is untouched by that recovery — it's still sitting in storage, and:

1. The new admin's only lever is to not call execute_upgrade (the malicious hash is public in the upgrade_proposed event, so they know not to). That's a passive mitigation, not a fix.
2. Because propose_upgrade unconditionally rejects with UpgradeAlreadyPending while any proposal exists, the new, legitimate admin can never propose a real upgrade — not this month, not ever — until this issue is fixed. The contract has no administrative path to clear stale upgrade state.

Reproduction steps

#[test]
fn test_stale_upgrade_survives_recovery_and_blocks_future_proposals() {
    let (env, _cid, admin, client) = setup();
    let g = [Address::generate(&env), Address::generate(&env), Address::generate(&env)];
    for guardian in &g { client.add_guardian(&admin, guardian); }
    client.set_recovery_config(&admin, &2, &10);

    // Compromised admin proposes a malicious upgrade.
    let malicious_hash = BytesN::from_array(&env, &[0xEEu8; 32]);
    client.propose_upgrade(&admin, &malicious_hash, &1);

    // Guardians successfully recover to a trusted new admin.
    let new_admin = Address::generate(&env);
    client.initiate_recovery(&g[0], &new_admin);
    client.approve_recovery(&g[1]);
    env.ledger().with_mut(|l| l.sequence_number += 20);
    client.execute_recovery();
    assert_eq!(client.admin(), new_admin);

    // The new, legitimate admin tries to propose a real upgrade later.
    let real_hash = BytesN::from_array(&env, &[0x11u8; 32]);
    let result = client.try_propose_upgrade(&new_admin, &real_hash, &100);
    assert_eq!(result, Err(Ok(WalletError::UpgradeAlreadyPending))); // blocked, forever, by the attacker's stale proposal
}

Impact

Beyond the immediate risk (a malicious upgrade proposal outliving the recovery that was supposed to neutralize the threat), this is a permanent, unrecoverable denial-of-service against the contract's own upgradeability — the mechanism meant to let the team ship fixes (including fixes to issues like this one) can be permanently disabled by a single malicious propose_upgrade call that nothing can ever clear.

Suggested fix

Add cancel_upgrade(env, admin) mirroring cancel_admin_transfer/cancel_recovery's shape (admin-authorized, clears PendingUpgrade, emits an event). Additionally, have execute_recovery clear PendingUpgrade the same way it already clears PendingAdmin for the outgoing admin — a successful emergency recovery should not leave the previous admin's unresolved proposals live against the new admin.

Definition of done

• cancel_upgrade function added, admin-authorized, symmetric with cancel_admin_transfer/cancel_recovery
• execute_recovery clears any pending PendingUpgrade as part of a successful recovery (documented as an explicit design decision, same as its existing PendingAdmin cleanup)
• Test proving a new admin can clear a stale/malicious upgrade proposal via cancel_upgrade
• Test proving a successful execute_recovery clears any in-flight PendingUpgrade automatically
• Test proving propose_upgrade works again for the new admin afterward (no more permanent lockout)
• cargo test --workspace output pasted

[Jumongweb]

Hi maintainer, i will like to solve this issue