Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

67 add remaining detector and vulnerability documentation #68

Merged
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,12 @@ Install Scout from the Marketplace within the Extensions tab of Visual Studio Co
| [overflow-check](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/overflow-check) | An arithmetic operation overflows or underflows the available memory allocated to the variable. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/overflow-check/overflow-check-1)| Critical |
| [insufficiently-random-values](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/insufficiently-random-values) | Avoid using block attributes for random number generation to prevent manipulation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/insufficiently-random-values/insufficiently-random-values-1)| Critical |
| [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical |
| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1)
| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhancement |
| [set-contract-storage](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/set-contract-storage) | Insufficient access control on `env.storage()` method. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-2), [3](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-3) | Critical |
| [avoid-panic-error](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-panic-error) | Code panics on error instead of using descriptive enum.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-panic-error/avoid-panic-error-1) | Enhancement |
| [avoid-unsafe-block](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-unsafe-block) | Using unsafe blocks in risks code safety and reliability.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-unsafe-block/avoid-unsafe-block-1) | Critical |
| [dos-unbounded-operation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dos-unbounded-operation) | DoS due to unbounded operation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-2), [3](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-3) | Medium |
| [soroban-version](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version) | Using a pinned version of Soroban can be dangerous, as it may have bugs or security issues. Use the latest version available. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1) | Enhancement |

## CLI Options

Expand Down
50 changes: 50 additions & 0 deletions detectors/avoid-panic-error/src/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Panic error

### What it does

The `panic!` macro is used to stop execution when a condition is not met.
This is useful for testing and prototyping, but should be avoided in production code.

### Why is this bad?

The usage of `panic!` is not recommended because it will stop the execution of the caller contract.

### Known problems

While this linter detects explicit calls to `panic!`, there are some ways to raise a panic such as `unwrap()` or `expect()`.

### Example

```rust
pub fn add(env: Env, value: u32) -> u32 {
let storage = env.storage().instance();
let mut count: u32 = storage.get(&COUNTER).unwrap_or(0);
match count.checked_add(value) {
Some(value) => count = value,
None => panic!("Overflow error"),
}
storage.set(&COUNTER, &count);
storage.extend_ttl(100, 100);
count
}
```

Use instead:

```rust
pub fn add(env: Env, value: u32) -> Result<u32, Error> {
let storage = env.storage().instance();
let mut count: u32 = storage.get(&COUNTER).unwrap_or(0);
match count.checked_add(value) {
Some(value) => count = value,
None => return Err(Error::OverflowError),
}
storage.set(&COUNTER, &count);
storage.extend_ttl(100, 100);
Ok(count)
}
```

### Implementation

The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-panic-error).
47 changes: 47 additions & 0 deletions detectors/avoid-unsafe-block/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Avoid unsafe block

### What it does

Checks for usage of `unsafe` blocks.

### Why is this bad?

`unsafe` blocks should not be used unless absolutely necessary. The use of unsafe blocks in Rust is discouraged because they bypass Rust's memory safety checks, potentially leading to issues like undefined behavior and security vulnerabilities.

### Example

```rust
pub fn unsafe_function(n: u64) -> u64 {
unsafe {
let mut i = n as f64;
let mut y = i.to_bits();
y = 0x5fe6ec85e7de30da - (y >> 1);
i = f64::from_bits(y);
i *= 1.5 - 0.5 * n as f64 * i * i;
i *= 1.5 - 0.5 * n as f64 * i * i;

let result_ptr: *mut f64 = &mut i;
let result = *result_ptr;

result.to_bits()
}
}
```

Use instead:

```rust
pub fn unsafe_function(n: u64) -> u64 {
let mut i = n as f64;
let mut y = i.to_bits();
y = 0x5fe6ec85e7de30da - (y >> 1);
i = f64::from_bits(y);
i *= 1.5 - 0.5 * n as f64 * i * i;
i *= 1.5 - 0.5 * n as f64 * i * i;
result.to_bits()
}
```

### Implementation

The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-unsafe-block).
43 changes: 43 additions & 0 deletions detectors/dos-unbounded-operation/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# DoS unbounded operation

### What it does

This detector checks that when using for or while loops, their conditions limit the execution to a constant number of iterations.

### Why is this bad?

If the number of iterations is not limited to a specific range, it could potentially cause out of gas exceptions.

### Known problems

False positives are to be expected when using variables that can only be set using controlled flows that limit the values within acceptable ranges.

### Example

```rust
pub fn unrestricted_loop(for_loop_count: u64) -> u64 {
let mut count = 0;
for i in 0..for_loop_count {
count += i;
}
count
}
```

Use instead:

```rust
const FIXED_COUNT: u64 = 1000;

pub fn restricted_loop_with_const() -> u64 {
let mut sum = 0;
for i in 0..FIXED_COUNT {
sum += i;
}
sum
}
```

### Implementation

The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dos-unbounded-operation).
25 changes: 25 additions & 0 deletions detectors/soroban-version/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Soroban version

### What it does

Warns you if you are using an old version of Soroban in the `Cargo.toml`.

### Why is this bad?

Using an old version of Soroban can be dangerous, as it may have bugs or security issues.

### Example

```toml
[dependencies]
soroban-sdk = { version = "=20.0.0" }

[dev_dependencies]
soroban-sdk = { version = "=20.0.0", features = ["testutils"] }
```

Instead, use the latest available version in the `Cargo.toml`.

### Implementation

The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version).
45 changes: 45 additions & 0 deletions test-cases/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,48 @@ We classified this type of vulnerability under
the [Authorization](#vulnerability-categories) category and assigned it a
Critical severity.

### Avoid panic error

The use of the `panic!` macro to stop execution when a condition is not met is
useful for testing and prototyping but should be avoided in production code.
Using `Result` as the return type for functions that can fail is the idiomatic
way to handle errors in Rust.

We classified this issue, a deviation from best practices which could have
security implications, under the [Validations and error handling](#vulnerability-categories) category and assigned it an Enhancement severity.

### Avoid unsafe block

The use of `unsafe` blocks in Rust is generally discouraged due to the potential risks it poses to the safety and reliability of the code. Rust's primary appeal lies in its ability to provide memory safety guarantees, which are largely enforced through its ownership and type systems. When you enter an `unsafe` block, you're effectively bypassing these safety checks. This can lead to various issues, such as undefined behavior, memory leaks, or security vulnerabilities. These blocks require the programmer to manually ensure that memory is correctly managed and accessed, which is prone to human error and can be challenging even for experienced developers. Therefore, unsafe blocks should only be used when absolutely necessary and when the safety of the operations within can be assured.

We classified this issue, a deviation from best practices which could have
security implications, under the [Validations and error handling](#vulnerability-categories) category and assigned it a Critical severity.

### DoS unbounded operation

Each block in Soroban Stellar has an upper bound on the amount of gas
that can be spent, and thus the amount of computation that can be done. This
is the Block Gas Limit. If the gas spent by a function call on a Soroban smart
contract exceeds this limit, the transaction will fail. Sometimes it is the
case that the contract logic allows a malicious user to modify conditions
so that other users are forced to exhaust gas on standard function calls.

In order to prevent a single transaction from consuming all the gas in a block,
unbounded operations must be avoided. This includes loops that do not have a
bounded number of iterations, and recursive calls.

A denial of service vulnerability allows the exploiter to hamper the
availability of a service rendered by the smart contract. In the context
of Soroban smart contracts, it can be caused by the exhaustion of gas,
storage space, or other failures in the contract's logic.

We classified this type of vulnerability under
the [Denial of Service](#vulnerability-categories) category and assigned it a
Medium severity.

### Soroban version

Using a pinned version of Soroban can be dangerous, as it may have bugs or security issues. Use the latest version available.

We classified this issue, a deviation from best practices which could have
security implications, under the [Best practices](#vulnerability-categories) category and assigned it an Enhancement severity.