Skip to content

Commit

Permalink
Merge pull request #178 from CoinFabrik/172-add-vulnerability-documen…
Browse files Browse the repository at this point in the history
…tation-for-6-unprotected-update-current-contract-wasm

172-add-vulnerability-documentation-for-6-unprotected-update-current-contract-wasm
  • Loading branch information
matiascabello authored Apr 25, 2024
2 parents f423a41 + 3a2fd4e commit 27107c9
Showing 1 changed file with 59 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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).

0 comments on commit 27107c9

Please sign in to comment.