From 6a3b0827a933956cda332d8ec9044a6bccea98ed Mon Sep 17 00:00:00 2001 From: tomasavola <108414862+tomasavola@users.noreply.github.com> Date: Thu, 8 Aug 2024 12:06:11 -0300 Subject: [PATCH 1/2] New assert-violation documentation --- docs/docs/detectors/15-assert-violation.md | 61 +++++++++++++++------- 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/docs/docs/detectors/15-assert-violation.md b/docs/docs/detectors/15-assert-violation.md index 09ca76c7..4c524e64 100644 --- a/docs/docs/detectors/15-assert-violation.md +++ b/docs/docs/detectors/15-assert-violation.md @@ -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) + +The `assert!` macro is used in Rust to ensure that a certain condition holds true at a certain point in your code. -### Why is this bad?​ +## Why is it bad? The `assert!` macro can cause the contract to panic. -### Example​ -```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 { - if value <= 10 { - Ok(true) - } else { - Err(AVError::GreaterThan10) - } - } + pub fn assert_if_greater_than_10(_env: Env, value: u128) -> Result { + 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) From 4d05142dcd1a8eb28f7d844d5b41452ceab19ee8 Mon Sep 17 00:00:00 2001 From: tomasavola <108414862+tomasavola@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:30:57 -0300 Subject: [PATCH 2/2] Add panic! explanation --- docs/docs/detectors/15-assert-violation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/detectors/15-assert-violation.md b/docs/docs/detectors/15-assert-violation.md index 4c524e64..87e39a97 100644 --- a/docs/docs/detectors/15-assert-violation.md +++ b/docs/docs/detectors/15-assert-violation.md @@ -11,7 +11,7 @@ The `assert!` macro is used in Rust to ensure that a certain condition holds tru ## Why is it bad? -The `assert!` macro can cause the contract to panic. +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. ## Issue example