From 3a2fd4ea4dc1b99bb3353a9dcb4b0ad90a667659 Mon Sep 17 00:00:00 2001 From: user Date: Thu, 25 Apr 2024 11:35:06 -0300 Subject: [PATCH] Add unprotected update current contract wasm vulnerability documentation --- ...nprotected-update-current-contract-wasm.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 docs/docs/vulnerabilities/6-unprotected-update-current-contract-wasm.md diff --git a/docs/docs/vulnerabilities/6-unprotected-update-current-contract-wasm.md b/docs/docs/vulnerabilities/6-unprotected-update-current-contract-wasm.md new file mode 100644 index 00000000..803af1db --- /dev/null +++ b/docs/docs/vulnerabilities/6-unprotected-update-current-contract-wasm.md @@ -0,0 +1,59 @@ +# Unprotected update current contract wasm + +## Description + +- Vulnerability Category: `Authorization` +- Vulnerability Severity: `Critical` +- Detectors: [`unprotected-update-current-contract-wasm`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) +- Test Cases: [`unprotected-update-current-contract-wasm-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1) [`unprotected-update-current-contract-wasm-2`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-2) + +It warns you if `update_current_contract_wasm()` function is called without a previous check of the address of the caller. If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour, leading to the loss of all associated data/tokens and functionalities given by this contract or by others that depend on it. + +## Exploit Scenario + +Consider the following `Soroban` contract: + +```rust +#[contractimpl] +impl UpgradeableContract { + pub fn init(e: Env, admin: Address) { + e.storage().instance().set(&DataKey::Admin, &admin); + } + + pub fn version() -> u32 { + 1 + } + + pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) { + e.deployer().update_current_contract_wasm(new_wasm_hash); + } +} +``` +This contract allows upgrades through the `update_current_contract_wasm` function. If just anyone can call this function, they could modify the contract behaviour. + +The vulnerable code example can be found [`here`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1/vulnerable-example). + +## Remediation + +To prevent this, the function should be restricted to administrators or authorized users only. + +```rust +#[contractimpl] +impl UpgradeableContract { + pub fn init(e: Env, admin: Address) { + e.storage().instance().set(&DataKey::Admin, &admin); + } + + pub fn version() -> u32 { + 1 + } + + pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) { + let admin: Address = e.storage().instance().get(&DataKey::Admin).unwrap(); + admin.require_auth(); + + e.deployer().update_current_contract_wasm(new_wasm_hash); + } +} +``` +The remediated code example can be found [`here`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1/remediated-example). \ No newline at end of file