Merge pull request #161 from CoinFabrik/156-add-documentation-unexpet…

…ed-revert-with-vector

156 add documentation unexpeted revert with vector

README.md

@@ -65,6 +65,7 @@ Visit [Scout's website](https://coinfabrik.github.io/scout-soroban/) to view the
 | [iterators-over-indexing](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/iterators-over-indexing)             |Iterating with hardcoded indexes is slower than using an iterator. Also, if the index is out of bounds, it will panic. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/iterators-over-indexing-1)                   | Enhancement  |
 | [assert-violation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/assert-violation)                           | Avoid the usage of the macro assert!, it can panic.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/assert-violation/assert-violation-1)                                                                            | Enhancement    |
 | [unprotected-mapping-operation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-mapping-operation)                           | Modifying mappings with an arbitrary key given by users can be a significant vulnerability. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-2)                   | Critical  |
+| [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  |
 
 
 ## Tests

docs/docs/detectors/17-dos-unexpected-revert-with-vector.md

@@ -0,0 +1,57 @@
+# DoS unexpected revert with vector
+
+### What it does
+
+Checks for array pushes without access control.
+
+### Why is this bad?
+
+Arrays have a maximum size according to the storage cell. If the array is full, the push will revert. This can be used to prevent the execution of a function.
+
+### Known problems
+
+If the owner validation is performed in an auxiliary function, the warning will be shown, resulting in a false positive.
+
+### Example
+
+```rust
+    pub fn add_candidate(env: Env, candidate: Address, caller: Address) -> Result<(), URError> {
+        caller.require_auth();
+        let mut state = Self::get_state(env.clone());
+        if Self::vote_ended(env.clone()) {
+            return Err(URError::VoteEnded);
+        }
+        if state.already_voted.contains_key(caller.clone()) {
+            return Err(URError::AccountAlreadyVoted); 
+        } else {
+            state.candidates.push_back(candidate.clone());
+            state.votes.set(candidate, 0);
+            Ok(())
+        }
+    }
+
+```
+
+Use instead:
+
+```rust
+ pub fn add_candidate(env: Env, candidate: Address, caller: Address) -> Result<(), URError> {
+      caller.require_auth();
+      let mut state = Self::get_state(env.clone());
+      if Self::vote_ended(env.clone()) {
+          return Err(URError::VoteEnded);
+      }
+      if Self::account_has_voted(env.clone(), caller.clone()) {
+          return Err(URError::AccountAlreadyVoted); 
+      } else {
+          env.storage().instance().set(&DataKey::Candidate(candidate.clone()), &Candidate{votes: 0});
+          state.total_candidates += 1; 
+          env.storage().instance().set(&DataKey::State, &state);
+          Ok(())
+      }
+}
+```
+
+### Implementation
+
+The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dos-unexpected-revert-with-vector).

docs/docs/vulnerabilities/README.md

@@ -222,3 +222,16 @@ Modifying mappings with an arbitrary key given by the user could lead to uninten
 
 This vulnerability falls under the [Validations and error handling category](#vulnerability-categories) and assigned it a Critical severity.
 
+### DoS unexpected revert with vector
+
+Another type of Denial of Service attack is called unexpected revert. It occurs
+by preventing transactions by other users from being successfully executed,
+forcing the blockchain state to revert to its original state.
+
+A Denial of Service through unexpected revert can
+accomplished by exploiting a smart contract that does not manage storage size
+errors correctly. It can be prevented by using Mapping instead of Vec to avoid
+storage limit problems.
+
+This vulnerability again falls under the [Denial of Service](#vulnerability-categories) category
+and has a Medium severity.