Require authorization in initialize() so the contract admin role cannot be front-run at deployment

[Cedarich]

• Complexity: Hard
• Labels: Soroban, contracts, security, Hard
• Overview: initialize() is an unprotected initializer. It checks has_admin() and then calls set_admin(&env, &admin) with no require_auth() on the address being installed and no authorisation check of any kind on the caller. Deployment and initialisation are separate transactions, so between the two anyone observing the ledger can call initialize() with their own address and become the contract's sole admin. Every other privileged entrypoint in the contract is correctly gated — initialize is the one that is not.
• Details:
  • initialize() spans roughly lib.rs:100-113. The first require_auth() in the file is at line 171, inside record_payment. There is no auth call anywhere in initialize.
  • The admin is not a nominal role. It is the only address that can call record_payment, allow_asset, revoke_asset, set_allow_native, set_paused, upgrade_storage, rebuild_history_index, and propose_admin. Taking it means taking the whole contract.
  • The outcome is unrecoverable. Once an attacker holds the admin role, initialize() returns AlreadyInitialized to the legitimate operator forever, and there is no reset path — propose_admin requires the current admin's authorisation, so the real operator cannot take the role back. With no WASM upgrade entrypoint either (Add an admin-gated WASM upgrade entrypoint to the invoice-payment contract #444), the deployment is permanently bricked and the only remedy is redeploying to a new contract ID.
  • An attacker who wins the race does not have to act loudly. They can install themselves, leave native XLM allowed, and let the backend fail every record_payment call with an authorisation error — or pause the contract and stall anchoring entirely.
  • The test suite cannot catch this. test.rs calls env.mock_all_auths() in 116 places, so every authorisation check is satisfied unconditionally. test_initialize_twice_returns_error covers the double-initialise path, but no test asserts that an unauthorised caller is rejected, because under mock_all_auths there is no such thing.
  • The deployment manifests in soroban/manifests/ and the ops scripts do not treat deploy-and-initialise as an atomic step, so the exposure window exists in the documented procedure rather than only in theory.
• Scope:
  • Require authorisation from the address being installed as admin in initialize(), so the role cannot be assigned to an address that did not consent to receive it.
  • Decide whether initialisation should additionally be constrained to the deployer, and implement whichever model is chosen deliberately rather than by omission.
  • Document the deploy-and-initialise sequence as a single atomic operation, and update the deployment scripts and manifests so the two steps cannot be separated by an attacker-observable gap.
  • Add tests that exercise real authorisation rather than mocked authorisation for every admin-gated entrypoint, so a missing require_auth fails a test instead of passing silently.
  • Audit the remaining entrypoints for the same class of omission now that mock_all_auths is understood to hide it.
  • Add a deployment verification step that confirms the expected admin is installed before the contract is put into service.
• Technical scope:
  • soroban/contracts/invoice-payment/src/lib.rs
  • soroban/contracts/invoice-payment/src/storage.rs
  • soroban/contracts/invoice-payment/src/test.rs
  • soroban/manifests/testnet.toml
  • soroban/manifests/mainnet.toml
  • soroban/client/src/soroban-invoice-client.ts
• Acceptance criteria:
  • initialize() fails unless the address being installed as admin authorises the call.
  • An account cannot install a different address as admin without that address's authorisation.
  • At least one test exercises the unauthorised-initialise path without mock_all_auths and asserts rejection.
  • Every admin-gated entrypoint has a test that asserts rejection under real authorisation, not mocked authorisation.
  • The deployment procedure initialises the contract atomically with deployment and verifies the resulting admin before the contract is used.
  • The audit of remaining entrypoints for missing authorisation checks is recorded, with any further findings fixed or filed.

[DatabyPrisca]

Please can you give me this issue? I have experience in this and I am sure to bring value to this project

[grantfox-oss]

🦊 GrantFox — @DatabyPrisca has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #479)
2. Your PR will be reviewed by the ZyntariHQ maintainers

Good luck! Track your progress on GrantFox.

[Cedarich]

Recycling this into the pool. PR #484 was closed with failing CI (ABI Drift Check and Test and Build), so no work landed here.

Reopened as: #529 — unassigned and open for anyone to pick up.