Merge pull request #164 from CoinFabrik/163-add-documentation-for-zer…

…o-or-test-address

163 add documentation for zero or test address

README.md

@@ -68,6 +68,7 @@ Visit [Scout's website](https://coinfabrik.github.io/scout-soroban/) to view the
 | [dos-unexpected-revert-with-vector](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dos-unexpected-revert-with-vector)                           | DoS due to improper storage. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unexpected-revert-with-vector/dos-unexpected-revert-with-vector-1)          | Medium  |
 | [unrestricted-transfer-from](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unrestricted-transfer-from)                           | Avoid passing an user-defined parameter as a `from` field in transfer-from. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unrestricted-transfer-from/unrestricted-transfer-from-1)                   | Critical  |
 | [unsafe-map-get](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-map-get)                           | Inappropriate usage of the `get` method for `Map` in soroban | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-map-get/unsafe-map-get-1)              | Medium |
+| [zero-or-test-address](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/zero-or-test-address)                           | Avoid zero or test address assignment to prevent contract control loss. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/zero-or-test-address/zero-or-test-address-1)                   | Validations and error handling  |
 
 
 ## Tests

docs/docs/detectors/20-zero-or-test-address

@@ -0,0 +1,43 @@
+# Zero or test address
+
+### What it does
+Checks whether the zero address is being inputed to a function without validation.
+
+### Why is this bad?
+Because the private key for the zero address is known, anyone could take ownership of the contract.
+
+### Example
+
+```rust
+pub fn set(e: Env, admin: Address, data: i32) -> Result<(), Error> {
+    if !ZeroAddressContract::ensure_is_admin(&e, admin)? {
+        return Err(Error::NotAdmin);
+    }
+    e.storage().persistent().set(&DataKey::Data, &data);
+    Ok(())
+}
+```
+
+
+Use instead:
+```rust
+pub fn set(e: Env, admin: Address, data: i32) -> Result<(), Error> {
+    if admin
+        == Address::from_string(&String::from_bytes(
+            &e,
+            b"GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF",
+        ))
+    {
+        return Err(Error::InvalidNewAdmin);
+    }
+    if !ZeroAddressContract::ensure_is_admin(&e, admin)? {
+        return Err(Error::NotAdmin);
+    }
+    e.storage().persistent().set(&DataKey::Data, &data);
+    Ok(())
+}
+```
+
+### Implementation
+
+The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/zero-or-test-address).

docs/docs/vulnerabilities/README.md

@@ -249,3 +249,11 @@ The use of certain methods (`get`, `get_unchecked`, `try_get_unchecked`) on a `M
 
 This vulnerability falls under the [Validations and error handling category](#vulnerability-categories) category and is assigned a Medium severity level.
 
+### Zero or test address
+
+The assignment of the zero address to a variable in a smart contract represents a critical vulnerability because it can lead to loss of control over the contract. This stems from the fact that the zero address does not have an associated private key, which means it's impossible to claim ownership, rendering any contract assets or functions permanently inaccessible.
+
+Assigning a test address can also have similar implications, including the loss of access or granting access to a malicious actor if its private keys are not handled with care.
+
+This vulnerability falls under the [Validations and error handling](#vulnerability-categories) category
+and has a Medium severity.