Skip to content

Commit

Permalink
Merge pull request #321 from CoinFabrik/migrate-unsafe-map-get-docume…
Browse files Browse the repository at this point in the history
…ntation

New unsafe-map-get documentation
  • Loading branch information
matiascabello authored Aug 11, 2024
2 parents 8c939e0 + 4da233c commit 464ad8b
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions docs/docs/detectors/19-unsafe-map-get.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
# Unsafe map get

### What it does
## Description

This detector identifies instances where unsafe methods like `get`, `get_unchecked`, and `try_get_unchecked` are used on `Map` objects in Soroban smart contracts.
- Category: `Validations and error handling`
- Severity: `Medium`
- Detectors: [`unsafe-map-get`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-map-get)
- Test Cases: [`unsafe-map-get-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-map-get/unsafe-map-get-1)

### Why is this bad?
The use of certain methods (`get`, `get_unchecked`, `try_get_unchecked`) on a `Map` object in the Soroban environment without appropriate error handling can lead to potential runtime panics. This issue stems from accessing the map's values with keys that may not exist, without using safer alternatives that check the existence of the key.

These methods are risky because they can lead to panics if the key does not exist in the map. Using these methods without proper checks increases the risk of runtime errors that can disrupt the execution of the smart contract and potentially lead to unexpected behavior or denial of service.
## Why is it bad?

### Example
These methods can lead to panics if the key does not exist in the map. Using these methods without proper checks increases the risk of runtime errors that can disrupt the execution of the smart contract and potentially lead to unexpected behavior or denial of service.

## Issue example

Consider the following `Soroban` contract:

```rust
pub fn get_from_map(env: Env) -> Option<i32> {
Expand All @@ -18,8 +25,9 @@ pub fn get_from_map(env: Env) -> Option<i32> {
map.get(1)
}
```
The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-map-get/unsafe-map-get-1/vulnerable-example).

Use instead:
## Remediated example

```rust
pub fn get_map_with_different_values(env: Env, key: i32) -> Result<Option<i32>, Error> {
Expand All @@ -34,7 +42,8 @@ pub fn get_map_with_different_values(env: Env, key: i32) -> Result<Option<i32>,
}
```

### Implementation
The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-map-get/unsafe-map-get-1/remediated-example).

The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-map-get).
## How is it detected?

Checks for array pushes without access control.

0 comments on commit 464ad8b

Please sign in to comment.