Merge pull request #45 from CoinFabrik/44-update-vulnerability-and-de…

…tector-documentation-for-milestone-1

44 update vulnerability and detector documentation for milestone 1
Closes #44

README.md

@@ -42,6 +42,8 @@ cargo scout-audit
 | [unsafe-unwrap](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-unwrap)      | Inappropriate usage of the unwrap method, causing unexpected program crashes.    | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-unwrap/unsafe-unwrap-1)| Medium    |
 | [unsafe-expect](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-expect)       | Improper usage of the expect method, leading to unexpected program crashes.    | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-expect/unsafe-expect-1)| Medium    |
 | [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    |
 
 
 ## Tests

detectors/unprotected-update-current-contract-wasm/README.md

@@ -0,0 +1,56 @@
+# Unprotected update of current contract wasm
+
+### What it does
+
+It warns you if `update_current_contract_wasm()` function is called without a previous check of the address of the caller.
+
+### Why is this bad?
+
+If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour, leading to the loss of all associated data/tokens and functionalities given by this contract or by others that depend on it.
+
+### Example
+
+```rust
+#[contractimpl]
+impl UpgradeableContract {
+    pub fn init(e: Env, admin: Address) {
+        e.storage().instance().set(&DataKey::Admin, &admin);
+    }
+
+    pub fn version() -> u32 {
+        1
+    }
+
+    pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) {
+        let admin: Address = e.storage().instance().get(&DataKey::Admin).unwrap();
+
+        e.deployer().update_current_contract_wasm(new_wasm_hash);
+    }
+}
+``` 
+
+Use instead:
+
+```rust
+#[contractimpl]
+impl UpgradeableContract {
+    pub fn init(e: Env, admin: Address) {
+        e.storage().instance().set(&DataKey::Admin, &admin);
+    }
+
+    pub fn version() -> u32 {
+        1
+    }
+
+    pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) {
+        let admin: Address = e.storage().instance().get(&DataKey::Admin).unwrap();
+        admin.require_auth();
+
+        e.deployer().update_current_contract_wasm(new_wasm_hash);
+    }
+}
+```
+
+### Implementation
+
+The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm)

test-cases/README.md

@@ -104,3 +104,17 @@ realized if `overflow-checks` is set to `False` in the `[profile.release]` secti
 Notwithstanding, there are contexts where developers do turn off checks for
 valid reasons and hence the reason for including this vulnerability in the
 list.
+
+
+### Insufficiently random values
+
+Using block attributes like ledger `timestamp()` and ledger `sequence()` for random number generation in Soroban smart contracts is not recommended due to the predictability of these values. Block attributes are publicly visible and deterministic, making it easy for malicious actors to anticipate their values and manipulate outcomes to their advantage. Furthermore, validators could potentially influence these attributes, further exacerbating the risk of manipulation. For truly random number generation, it's important to use a source that is both unpredictable and external to the blockchain environment, reducing the potential for malicious exploitation.
+
+This vulnerability again falls under the [Block attributes](#vulnerability-categories) category
+and has a Critical severity.
+
+### Unprotected update of current contract wasm
+
+If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour, leading to the loss of all associated data/tokens and functionalities given by this contract or by others that depend on it. To prevent this, the function should be restricted to administrators or authorized users only.
+
+This vulnerability falls under the [Authorization](#vulnerability-categories) category and has a Critical severity.