From d4cc35751f5f923e65ba3dc3d561e37fe06caacb Mon Sep 17 00:00:00 2001
From: tomasavola <108414862+tomasavola@users.noreply.github.com>
Date: Wed, 7 Aug 2024 09:48:07 -0300
Subject: [PATCH] New unsafe-unwrap documentation

---
 docs/docs/detectors/2-unsafe-unwrap.md | 81 +++++++++++++++++++-------
 1 file changed, 59 insertions(+), 22 deletions(-)

diff --git a/docs/docs/detectors/2-unsafe-unwrap.md b/docs/docs/detectors/2-unsafe-unwrap.md
index a49f21b5..56dbb8f0 100644
--- a/docs/docs/detectors/2-unsafe-unwrap.md
+++ b/docs/docs/detectors/2-unsafe-unwrap.md
@@ -1,39 +1,76 @@
 # Unsafe unwrap
 
-### What it does
+## Description 
 
-Checks for usage of `.unwrap()`
+- Category: `Validations and error handling`
+- Severity: `Minor`
+- Detectors: [`unsafe-unwrap`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-unwrap)
+- Test Cases: [`unsafe-unwrap-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-1) [`unsafe-unwrap-2`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-2) [`unsafe-unwrap-3`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-3)  [`unsafe-unwrap-4`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-4)  [`unsafe-unwrap-5`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-5)  [`unsafe-unwrap-6`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-6) 
 
-### Why is this bad?
 
-`.unwrap()` might panic if the result value is an error or `None`.
+In Rust, the `unwrap` method is commonly used for error handling. It retrieves the inner value of an `Option` or `Result`. If an error or `None` occurs, it calls `panic!` without a custom error message.
 
-### Example
+## Why is this bad? 
+
+`.unwrap()` might panic if the result value is an error or `None`. It is recommended to avoid the panic of a contract 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 
+
+Consider the following `Soroban` contract:
 
 ```rust
-// example code where a warning is issued
-fn main() {
-    let result = result_fn().unwrap("error");
-}
-fn result_fn() -> Result<u8, Error> {
-    Err(Error::new(ErrorKind::Other, "error"))
+#[contractimpl]
+impl UnsafeUnwrap {
+    pub fn unwrap(n: u64) -> u64 {
+        let result = Self::non_zero_or_error(n);
+        result.unwrap()
+    }
+
+    pub fn non_zero_or_error(n: u64) -> Result<u64, Error> {
+        if n == 0 {
+            return Err(Error::CustomError);
+        }
+        Ok(n)
+    }
 }
 ```
 
-Use instead:
+In this contract, the `unwrap` function uses the `unwrap` method to save the result of the `non_zero_or_error` function. If the function returns `Err`, the contract will panic and halt execution, potentially leading to malicious exploitation to disrupt the contract's operation.
+
+The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-1/vulnerable-example).
+
+## Remediated example
+
+Instead of using `unwrap`, use a safer method for error handling. In this case, if the function returns `Err`, it will return a default value (like `0`).
 
 ```rust
-// example code that does not raise a warning
-fn main() {
-   let result = if let Ok(result) = result_fn() {
-      result
-  }
-}
-fn result_fn() -> Result<u8, Error> {
-    Err(Error::new(ErrorKind::Other, "error"))
+#[contractimpl]
+impl UnsafeUnwrap {
+    pub fn unwrap_or_default(n: u64) -> u64 {
+        let result = Self::non_zero_or_error(n);
+        result.unwrap_or(0)
+    }
+
+    pub fn non_zero_or_error(n: u64) -> Result<u64, Error> {
+        if n == 0 {
+            return Err(Error::CustomError);
+        }
+        Ok(n)
+    }
 }
 ```
 
-### Implementation
+The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-1/remediated-example).
+
+## How is it detected?
+
+Checks for usage of .unwrap()
+
+## References
+
+[Rust documentation: `unwrap`](https://doc.rust-lang.org/std/option/enum.Option.html#method.unwrap)
+
+
+
 
-The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-unwrap).
+