-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
New set-contract-storage documentation
- Loading branch information
1 parent
53157fd
commit cd32c51
Showing
1 changed file
with
56 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,74 @@ | ||
# Set contract storage | ||
|
||
### What it does | ||
## Description | ||
|
||
Checks for calls to `env.storage()` without a prior call to `env.require_auth()`. | ||
- Category: `Authorization` | ||
- Severity: `Critical` | ||
- Detector: [`set-contract-storage`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/set-contract-storage) | ||
- Test Cases: [`set-contract-storage-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-1) [`set-contract-storage-2`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-2) [`set-contract-storage-3`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-3) [`set-contract-storage-4`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-4) | ||
|
||
### Why is this bad? | ||
Smart contracts can store important information in memory which changes through the contract's lifecycle. Changes happen via user interaction with the smart contract. An unauthorized set contract storage issue happens when a smart contract call allows a user to set or modify contract memory when he was not supposed to be authorized. | ||
|
||
Functions using keys as variables without proper access control or input sanitation can allow users to perform changes in arbitrary memory locations. | ||
## Why is this bad? | ||
|
||
### Known problems | ||
Unauthorized access to storage by a user can lead to serious issues, as they might manipulate memory data to attack the contract. | ||
|
||
Only check the function call, so false positives could result. | ||
## Issue example | ||
|
||
### Example | ||
Consider the following `Soroban` contract: | ||
|
||
```rust | ||
fn set_contract_storage(env: Env) { | ||
let _storage = env.storage().instance(); | ||
} | ||
|
||
#[contractimpl] | ||
impl SetContractStorage { | ||
/// Increment an internal counter; return the new value. | ||
pub fn increment(env: Env, user: Address) -> u32 { | ||
let storage = env.storage().instance(); | ||
let mut count: u32 = storage.get(&user).unwrap_or_default(); | ||
count += 1; | ||
storage.set(&user, &count); | ||
storage.extend_ttl(100, 100); | ||
count | ||
} | ||
} | ||
|
||
``` | ||
|
||
Use instead: | ||
In this example we see that any user may access the SetContractStorage() function, and therefore modify the value of the internal counter. | ||
|
||
The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-1/vulnerable-example). | ||
|
||
|
||
## Remediated example | ||
|
||
Arbitrary users should not have control over keys because it implies writing any value of a mapping, lazy variable, or the main struct of the contract located in position 0 of the storage. To prevent this issue, set access control and proper authorization validation for the SetContractStorage() function. | ||
|
||
For example, the code below, ensures only the authorized users can call SetContractStorage(). | ||
|
||
```rust | ||
fn set_contract_storage(env: Env, user: Address) { | ||
user.require_auth(); | ||
let _storage = env.storage().instance(); | ||
|
||
#[contractimpl] | ||
impl SetContractStorage { | ||
/// Increment an internal counter; return the new value. | ||
pub fn increment(env: Env, user: Address) -> u32 { | ||
user.require_auth(); | ||
let storage = env.storage().instance(); | ||
let mut count: u32 = storage.get(&user).unwrap_or_default(); | ||
count += 1; | ||
storage.set(&user, &count); | ||
storage.extend_ttl(100, 100); | ||
count | ||
} | ||
} | ||
|
||
``` | ||
|
||
### Implementation | ||
The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-1/remediated-example). | ||
|
||
## How is it detected? | ||
|
||
Checks for calls to env.storage() without a prior call to env.require_auth(). | ||
|
||
|
||
|
||
The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/set-contract-storage). | ||