diff --git a/detectors/dynamic-instance-storage/Cargo.toml b/detectors/dynamic-storage/Cargo.toml similarity index 89% rename from detectors/dynamic-instance-storage/Cargo.toml rename to detectors/dynamic-storage/Cargo.toml index 3060bf9f..c564b4ff 100644 --- a/detectors/dynamic-instance-storage/Cargo.toml +++ b/detectors/dynamic-storage/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2021" -name = "dynamic-instance-storage" +name = "dynamic-storage" version = "0.1.0" [lib] diff --git a/detectors/dynamic-instance-storage/src/lib.rs b/detectors/dynamic-storage/src/lib.rs similarity index 74% rename from detectors/dynamic-instance-storage/src/lib.rs rename to detectors/dynamic-storage/src/lib.rs index 9319f082..151a0c52 100644 --- a/detectors/dynamic-instance-storage/src/lib.rs +++ b/detectors/dynamic-storage/src/lib.rs @@ -15,26 +15,26 @@ use rustc_middle::ty::{Ty, TyKind}; use rustc_span::{def_id::LocalDefId, Span, Symbol}; use utils::{get_node_type_opt, is_soroban_storage, SorobanStorageType}; -const LINT_MESSAGE: &str = "This function may lead to excessive instance storage growth, which could increase execution costs or potentially cause DoS"; +const LINT_MESSAGE: &str = "Using dynamic types in instance or persistent storage can lead to unnecessary growth or storage-related vulnerabilities."; dylint_linting::impl_late_lint! { - pub DYNAMIC_INSTANCE_STORAGE, + pub DYNAMIC_STORAGE, Warn, LINT_MESSAGE, - DynamicInstanceStorage, + DynamicStorage, { - name: "Dynamic Instance Storage Analyzer", - long_message: "Detects potential misuse of instance storage that could lead to unnecessary growth or storage-related vulnerabilities.", + name: "Dynamic Storage Analyzer", + long_message: "Using dynamic types in instance or persistent storage can lead to unnecessary growth or storage-related vulnerabilities.", severity: "Warning", - help: "https://coinfabrik.github.io/scout-soroban/docs/detectors/dynamic-instance-storage", + help: "https://coinfabrik.github.io/scout-soroban/docs/detectors/dynamic-storage", vulnerability_class: "Resource Management", } } #[derive(Default)] -struct DynamicInstanceStorage; +struct DynamicStorage; -impl<'tcx> LateLintPass<'tcx> for DynamicInstanceStorage { +impl<'tcx> LateLintPass<'tcx> for DynamicStorage { fn check_fn( &mut self, cx: &LateContext<'tcx>, @@ -48,30 +48,31 @@ impl<'tcx> LateLintPass<'tcx> for DynamicInstanceStorage { return; } - let mut storage_warn_visitor = DynamicInstanceStorageVisitor { cx }; + let mut storage_warn_visitor = DynamicStorageVisitor { cx }; storage_warn_visitor.visit_body(body); } } -struct DynamicInstanceStorageVisitor<'a, 'tcx> { +struct DynamicStorageVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, } -impl<'a, 'tcx> Visitor<'tcx> for DynamicInstanceStorageVisitor<'a, 'tcx> { +impl<'a, 'tcx> Visitor<'tcx> for DynamicStorageVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { if_chain! { // Detect calls to `set` method if let ExprKind::MethodCall(path, receiver, args, _) = &expr.kind; if path.ident.name == Symbol::intern("set"); - // Get the type of the receiver and check if it is an instance storage + // Get the type of the receiver and check if it is an instance or persistent storage if let Some(receiver_ty) = get_node_type_opt(self.cx, &receiver.hir_id); - if is_soroban_storage(self.cx, receiver_ty, SorobanStorageType::Instance); + if is_soroban_storage(self.cx, receiver_ty, SorobanStorageType::Instance) + || is_soroban_storage(self.cx, receiver_ty, SorobanStorageType::Persistent); // Check if the value being set is a dynamic type if args.len() >= 2; if let Some(value_type) = get_node_type_opt(self.cx, &args[1].hir_id); if is_dynamic_type(self.cx, &value_type); then { - span_lint(self.cx, DYNAMIC_INSTANCE_STORAGE, expr.span, LINT_MESSAGE) + span_lint(self.cx, DYNAMIC_STORAGE, expr.span, LINT_MESSAGE) } } diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/src/lib.rs b/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/src/lib.rs deleted file mode 100644 index 10f79de7..00000000 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/src/lib.rs +++ /dev/null @@ -1,43 +0,0 @@ -#![no_std] -use soroban_sdk::{contract, contractimpl, Env, Symbol, Vec}; - -#[contract] -pub struct VectorStorage; - -#[contractimpl] -impl VectorStorage { - pub fn store_vector(e: Env, data: Vec) { - e.storage() - .instance() - .set(&Symbol::new(&e, "vector_data"), &data); - } - - pub fn get_vector(e: Env) -> Vec { - e.storage() - .instance() - .get(&Symbol::new(&e, "vector_data")) - .unwrap() - } -} - -#[cfg(test)] -mod test { - use super::*; - use soroban_sdk::{vec, Env}; - - #[test] - fn test_vector_storage() { - // Given - let env = Env::default(); - let contract_id = env.register_contract(None, VectorStorage); - let client = VectorStorageClient::new(&env, &contract_id); - - // When - let test_vec = vec![&env, 1, 2, 3, 4, 5]; - client.store_vector(&test_vec); - - // Then - let retrieved_vec = client.get_vector(); - assert_eq!(test_vec, retrieved_vec); - } -} diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/src/lib.rs b/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/src/lib.rs deleted file mode 100644 index 354b0abc..00000000 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/src/lib.rs +++ /dev/null @@ -1,43 +0,0 @@ -#![no_std] -use soroban_sdk::{contract, contractimpl, Env, String, Symbol}; - -#[contract] -pub struct StringStorage; - -#[contractimpl] -impl StringStorage { - pub fn store_string(e: Env, data: String) { - e.storage() - .persistent() - .set(&Symbol::new(&e, "string_data"), &data); - } - - pub fn get_string(e: Env) -> String { - e.storage() - .persistent() - .get(&Symbol::new(&e, "string_data")) - .unwrap() - } -} - -#[cfg(test)] -mod test { - use super::*; - use soroban_sdk::Env; - - #[test] - fn test_string_storage() { - // Given - let env = Env::default(); - let contract_id = env.register_contract(None, StringStorage); - let client = StringStorageClient::new(&env, &contract_id); - - // When - let test_string = String::from_str(&env, "Hello, Soroban!"); - client.store_string(&test_string); - - // Then - let retrieved_string = client.get_string(); - assert_eq!(test_string, retrieved_string); - } -} diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/src/lib.rs b/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/src/lib.rs deleted file mode 100644 index 79a36aa0..00000000 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/src/lib.rs +++ /dev/null @@ -1,43 +0,0 @@ -#![no_std] -use soroban_sdk::{contract, contractimpl, Env, String, Symbol}; - -#[contract] -pub struct StringStorage; - -#[contractimpl] -impl StringStorage { - pub fn store_string(e: Env, data: String) { - e.storage() - .instance() - .set(&Symbol::new(&e, "string_data"), &data); - } - - pub fn get_string(e: Env) -> String { - e.storage() - .instance() - .get(&Symbol::new(&e, "string_data")) - .unwrap() - } -} - -#[cfg(test)] -mod test { - use super::*; - use soroban_sdk::Env; - - #[test] - fn test_string_storage() { - // Given - let env = Env::default(); - let contract_id = env.register_contract(None, StringStorage); - let client = StringStorageClient::new(&env, &contract_id); - - // When - let test_string = String::from_str(&env, "Hello, Soroban!"); - client.store_string(&test_string); - - // Then - let retrieved_string = client.get_string(); - assert_eq!(test_string, retrieved_string); - } -} diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/remediated-example/Cargo.toml b/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/remediated-example/Cargo.toml deleted file mode 100644 index fa1d3059..00000000 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/remediated-example/Cargo.toml +++ /dev/null @@ -1,16 +0,0 @@ -[package] -edition = "2021" -name = "dynamic-instance-storage-remediated-3" -version = "0.1.0" - -[lib] -crate-type = ["cdylib"] - -[dependencies] -soroban-sdk = { workspace = true } - -[dev-dependencies] -soroban-sdk = { workspace = true, features = ["testutils"] } - -[features] -testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/vulnerable-example/Cargo.toml b/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/vulnerable-example/Cargo.toml deleted file mode 100644 index 5f0d3c3d..00000000 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/vulnerable-example/Cargo.toml +++ /dev/null @@ -1,16 +0,0 @@ -[package] -edition = "2021" -name = "dynamic-instance-storage-vulnerable-3" -version = "0.1.0" - -[lib] -crate-type = ["cdylib"] - -[dependencies] -soroban-sdk = { workspace = true } - -[dev-dependencies] -soroban-sdk = { workspace = true, features = ["testutils"] } - -[features] -testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/dynamic-instance-storage/Cargo.toml b/test-cases/dynamic-storage/Cargo.toml similarity index 79% rename from test-cases/dynamic-instance-storage/Cargo.toml rename to test-cases/dynamic-storage/Cargo.toml index 2af55099..6ac24264 100644 --- a/test-cases/dynamic-instance-storage/Cargo.toml +++ b/test-cases/dynamic-storage/Cargo.toml @@ -1,10 +1,10 @@ [workspace] exclude = [".cargo", "target"] -members = ["dynamic-instance-storage-*/*"] +members = ["dynamic-storage-*/*"] resolver = "2" [workspace.dependencies] -soroban-sdk = { version = "=21.4.0" } +soroban-sdk = { version = "=21.5.1" } [profile.release] codegen-units = 1 diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/Cargo.toml b/test-cases/dynamic-storage/dynamic-storage-1/remediated-example/Cargo.toml similarity index 84% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/Cargo.toml rename to test-cases/dynamic-storage/dynamic-storage-1/remediated-example/Cargo.toml index 9241c0f4..f096e323 100644 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/Cargo.toml +++ b/test-cases/dynamic-storage/dynamic-storage-1/remediated-example/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2021" -name = "dynamic-instance-storage-vulnerable-2" +name = "dynamic-storage-remediated-1" version = "0.1.0" [lib] diff --git a/test-cases/dynamic-storage/dynamic-storage-1/remediated-example/src/lib.rs b/test-cases/dynamic-storage/dynamic-storage-1/remediated-example/src/lib.rs new file mode 100644 index 00000000..db7a3faa --- /dev/null +++ b/test-cases/dynamic-storage/dynamic-storage-1/remediated-example/src/lib.rs @@ -0,0 +1,63 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Env, Vec}; + +#[contract] +pub struct VectorStorage; + +#[derive(Clone)] +#[contracttype] +pub enum DataKey { + VecElement(u32), +} + +#[contractimpl] +impl VectorStorage { + 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); + } + } + + pub fn get_vector(e: Env) -> Vec { + let mut result = Vec::new(&e); + let mut i = 0; + + while let Some(value) = VectorStorage::get_element(e.clone(), i) { + result.push_back(value); + i += 1; + } + + result + } + + pub fn get_element(e: Env, index: u32) -> Option { + let key = DataKey::VecElement(index); + e.storage().persistent().get(&key) + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::{vec, Env}; + + #[test] + fn test_vector_storage() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, VectorStorage); + let client = VectorStorageClient::new(&env, &contract_id); + + // When + let test_vec = vec![&env, 1, 2, 3, 4, 5]; + client.store_vector(&test_vec); + + // Then + let retrieved_vec = client.get_vector(); + assert_eq!(test_vec, retrieved_vec); + + assert_eq!(client.get_element(&2), Some(3)); + assert_eq!(client.get_element(&5), None); + } +} diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/Cargo.toml b/test-cases/dynamic-storage/dynamic-storage-1/vulnerable-example/Cargo.toml similarity index 84% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/Cargo.toml rename to test-cases/dynamic-storage/dynamic-storage-1/vulnerable-example/Cargo.toml index 7122885e..f1ef1214 100644 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/Cargo.toml +++ b/test-cases/dynamic-storage/dynamic-storage-1/vulnerable-example/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2021" -name = "dynamic-instance-storage-vulnerable-1" +name = "dynamic-storage-vulnerable-1" version = "0.1.0" [lib] diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/src/lib.rs b/test-cases/dynamic-storage/dynamic-storage-1/vulnerable-example/src/lib.rs similarity index 100% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/src/lib.rs rename to test-cases/dynamic-storage/dynamic-storage-1/vulnerable-example/src/lib.rs diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/Cargo.toml b/test-cases/dynamic-storage/dynamic-storage-2/remediated-example/Cargo.toml similarity index 84% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/Cargo.toml rename to test-cases/dynamic-storage/dynamic-storage-2/remediated-example/Cargo.toml index 7d90d213..e0bfcd9c 100644 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/Cargo.toml +++ b/test-cases/dynamic-storage/dynamic-storage-2/remediated-example/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2021" -name = "dynamic-instance-storage-remediated-2" +name = "dynamic-storage-remediated-2" version = "0.1.0" [lib] diff --git a/test-cases/dynamic-storage/dynamic-storage-2/remediated-example/src/lib.rs b/test-cases/dynamic-storage/dynamic-storage-2/remediated-example/src/lib.rs new file mode 100644 index 00000000..de7647e6 --- /dev/null +++ b/test-cases/dynamic-storage/dynamic-storage-2/remediated-example/src/lib.rs @@ -0,0 +1,44 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, Env, Map, Symbol}; + +#[contract] +pub struct MapStorage; + +#[contractimpl] +impl MapStorage { + pub fn store_map(e: Env, data: Map) { + data.iter().for_each(|(key, value)| { + e.storage().persistent().set(&key, &value); + }); + } + + pub fn get_key(e: Env, key: Symbol) -> i32 { + e.storage().persistent().get(&key).unwrap() + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::{symbol_short, Env, Map}; + + #[test] + fn test_map_storage() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, MapStorage); + let client = MapStorageClient::new(&env, &contract_id); + + // When + let mut test_map = Map::new(&env); + test_map.set(symbol_short!("key1"), 1); + test_map.set(symbol_short!("key2"), 2); + client.store_map(&test_map); + + // Then + let key1 = client.get_key(&symbol_short!("key1")); + let key2 = client.get_key(&symbol_short!("key2")); + assert_eq!(test_map.get(symbol_short!("key1")).unwrap(), key1); + assert_eq!(test_map.get(symbol_short!("key2")).unwrap(), key2); + } +} diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/Cargo.toml b/test-cases/dynamic-storage/dynamic-storage-2/vulnerable-example/Cargo.toml similarity index 84% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/Cargo.toml rename to test-cases/dynamic-storage/dynamic-storage-2/vulnerable-example/Cargo.toml index 6da39cf1..93954019 100644 --- a/test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/Cargo.toml +++ b/test-cases/dynamic-storage/dynamic-storage-2/vulnerable-example/Cargo.toml @@ -1,6 +1,6 @@ [package] edition = "2021" -name = "dynamic-instance-storage-remediated-1" +name = "dynamic-storage-vulnerable-2" version = "0.1.0" [lib] diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/remediated-example/src/lib.rs b/test-cases/dynamic-storage/dynamic-storage-2/vulnerable-example/src/lib.rs similarity index 100% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-3/remediated-example/src/lib.rs rename to test-cases/dynamic-storage/dynamic-storage-2/vulnerable-example/src/lib.rs diff --git a/test-cases/dynamic-storage/dynamic-storage-3/remediated-example/Cargo.toml b/test-cases/dynamic-storage/dynamic-storage-3/remediated-example/Cargo.toml new file mode 100644 index 00000000..32465e65 --- /dev/null +++ b/test-cases/dynamic-storage/dynamic-storage-3/remediated-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "dynamic-storage-remediated-3" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/dynamic-storage/dynamic-storage-3/remediated-example/src/lib.rs b/test-cases/dynamic-storage/dynamic-storage-3/remediated-example/src/lib.rs new file mode 100644 index 00000000..60f47e41 --- /dev/null +++ b/test-cases/dynamic-storage/dynamic-storage-3/remediated-example/src/lib.rs @@ -0,0 +1,44 @@ +#![no_std] +use soroban_sdk::{contract, contractimpl, Env, Map, Symbol}; + +#[contract] +pub struct MapStorage; + +#[contractimpl] +impl MapStorage { + pub fn store_map(e: Env, data: Map) { + data.iter().for_each(|(key, value)| { + e.storage().instance().set(&key, &value); + }); + } + + pub fn get_key(e: Env, key: Symbol) -> i32 { + e.storage().instance().get(&key).unwrap() + } +} + +#[cfg(test)] +mod test { + use super::*; + use soroban_sdk::{symbol_short, Env, Map}; + + #[test] + fn test_map_storage() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, MapStorage); + let client = MapStorageClient::new(&env, &contract_id); + + // When + let mut test_map = Map::new(&env); + test_map.set(symbol_short!("key1"), 1); + test_map.set(symbol_short!("key2"), 2); + client.store_map(&test_map); + + // Then + let key1 = client.get_key(&symbol_short!("key1")); + let key2 = client.get_key(&symbol_short!("key2")); + assert_eq!(test_map.get(symbol_short!("key1")).unwrap(), key1); + assert_eq!(test_map.get(symbol_short!("key2")).unwrap(), key2); + } +} diff --git a/test-cases/dynamic-storage/dynamic-storage-3/vulnerable-example/Cargo.toml b/test-cases/dynamic-storage/dynamic-storage-3/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..37605628 --- /dev/null +++ b/test-cases/dynamic-storage/dynamic-storage-3/vulnerable-example/Cargo.toml @@ -0,0 +1,16 @@ +[package] +edition = "2021" +name = "dynamic-storage-vulnerable-3" +version = "0.1.0" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/test-cases/dynamic-instance-storage/dynamic-instance-storage-3/vulnerable-example/src/lib.rs b/test-cases/dynamic-storage/dynamic-storage-3/vulnerable-example/src/lib.rs similarity index 100% rename from test-cases/dynamic-instance-storage/dynamic-instance-storage-3/vulnerable-example/src/lib.rs rename to test-cases/dynamic-storage/dynamic-storage-3/vulnerable-example/src/lib.rs