-
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.
Merge pull request #317 from CoinFabrik/migrate-assert-violation-docu…
…mentation New assert-violation documentation
- Loading branch information
Showing
1 changed file
with
42 additions
and
21 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,33 +1,54 @@ | ||
# Assert violation | ||
# Assert violation | ||
|
||
### What it does | ||
## Description | ||
|
||
Checks for `assert!` macro usage. | ||
- Category: `Validations and error handling` | ||
- Severity: `Enhancement` | ||
- Detector: [`assert-violation`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/assert-violation) | ||
- Test Cases: [`assert-violation-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/assert-violation/assert-violation-1) | ||
|
||
### Why is this bad? | ||
The `assert!` macro is used in Rust to ensure that a certain condition holds true at a certain point in your code. | ||
|
||
The `assert!` macro can cause the contract to panic. | ||
## Why is it bad? | ||
|
||
### Example | ||
The `assert!` macro can cause the contract to panic. It is recommended to avoid this, because it stops its execution, which might lead the contract to an inconsistent state if the panic occurs in the middle of state changes. Additionally, the panic could cause a transaction to fail. | ||
|
||
```rust | ||
pub fn assert_if_greater_than_10(_env: Env, value: u128) -> bool { | ||
assert!(value <= 10, "value should be less than 10"); | ||
true | ||
} | ||
|
||
## Issue example | ||
|
||
Consider the following `Soroban` contract: | ||
|
||
```rust | ||
pub fn assert_if_greater_than_10(_env: Env, value: u128) -> bool { | ||
assert!(value <= 10, "value should be less than 10"); | ||
true | ||
} | ||
``` | ||
Use instead: | ||
|
||
The problem arises from the use of the `assert!` macro, if the condition is not met, the contract panics. | ||
|
||
The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/assert-violation/assert-violation-1/vulnerable-example). | ||
|
||
## Remediated example | ||
|
||
Avoid the use of `assert!` macro. Instead, use a proper error and return it. | ||
|
||
```rust | ||
pub fn assert_if_greater_than_10(_env: Env, value: u128) -> Result<bool, AVError> { | ||
if value <= 10 { | ||
Ok(true) | ||
} else { | ||
Err(AVError::GreaterThan10) | ||
} | ||
} | ||
pub fn assert_if_greater_than_10(_env: Env, value: u128) -> Result<bool, AVError> { | ||
if value <= 10 { | ||
Ok(true) | ||
} else { | ||
Err(AVError::GreaterThan10) | ||
} | ||
} | ||
``` | ||
### Implementation | ||
|
||
The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/assert-violation). | ||
The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/assert-violation/assert-violation-1/remediated-example). | ||
|
||
## How is it detected? | ||
|
||
Checks for `assert!` macro usage. | ||
|
||
## References | ||
|
||
- [Assert violation](https://docs.alephzero.org/aleph-zero/security-course-by-kudelski-security/ink-developers-security-guideline#assert-violation) |