Merge pull request #68 from CoinFabrik/67-add-remaining-detector-and-…

…vulnerability-documentation

67 add remaining detector and vulnerability documentation

README.md

@@ -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
 

detectors/avoid-panic-error/src/README.md

@@ -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).

detectors/avoid-unsafe-block/README.md

@@ -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).

detectors/dos-unbounded-operation/README.md

@@ -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).

detectors/soroban-version/README.md

@@ -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).

test-cases/README.md

@@ -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.