From 747a208f7d638f9be3b7877d20fc2fb38dc68fc3 Mon Sep 17 00:00:00 2001 From: tomasavola <108414862+tomasavola@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:05:56 -0300 Subject: [PATCH] add 29-dynamic-types-in-storage --- .../detectors/29-dynamic-types-in-storage | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 docs/docs/detectors/29-dynamic-types-in-storage diff --git a/docs/docs/detectors/29-dynamic-types-in-storage b/docs/docs/detectors/29-dynamic-types-in-storage new file mode 100644 index 00000000..1186102d --- /dev/null +++ b/docs/docs/detectors/29-dynamic-types-in-storage @@ -0,0 +1,52 @@ +# Dynamic types in storage + +## Description + +- Category: `Authorization` +- Severity: `Critical` +- Detectors: [`dynamic-instance-storage`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dynamic-instance-storage) +- Test Cases: [`dynamic-instance-storage-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dynamic-instance-storage/dynamic-instance-storage-1) [`dynamic-instance-storage-2`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dynamic-instance-storage/dynamic-instance-storage-2) [`dynamic-instance-storage-3`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dynamic-instance-storage/dynamic-instance-storage-3) + +In Rust, it is very useful to use `storage.persistent()` to store data that is shared among all users of the contract (e.g., a token administrator). However, using this macro with dynamic variables (such as vectors, maps, etc.) is not recommended. + +## Why is this bad? + +Using dynamic values with `storage.persistent()` can cause excessive storage use and may risk DoS attacks on the contract. + +## Issue example + +Consider the following `Soroban` contract: + +```rust + + pub fn store_vector(e: Env, data: Vec) { + e.storage() + .persistent() + .set(&Symbol::new(&e, "vector_data"), &data); + } + +``` +In this example, the function is storing a vector using `storage.persistent()`. + +The code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example). + +## Remediated example + +Consider the following `Soroban` contract: + +```rust + pub fn store_vector(e: Env, data: Vec) { + for (i, value) in data.iter().enumerate() { + let key = DataKey::VecElement(i as u32); + e.storage().persistent().set(&key, &value); + } + } +``` + +Instead of using `storage.persistent()` to store a vector in the storage, the data belonging to the vector can be stored directly in the storage. + +The remediated code example can be found [here](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example). + +## How is it detected? + +Checks the usage of `storage().persistent()` with dynamic types.