287 inadequate use of instance storage

detectors/dynamic-instance-storage/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+edition = "2021"
+name = "dynamic-instance-storage"
+version = "0.1.0"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+clippy_utils = { workspace = true }
+dylint_linting = { workspace = true }
+if_chain = { workspace = true }
+utils = { workspace = true }
+
+[package.metadata.rust-analyzer]
+rustc_private = true

detectors/dynamic-instance-storage/src/lib.rs

@@ -0,0 +1,97 @@
+#![feature(rustc_private)]
+
+extern crate rustc_hir;
+extern crate rustc_middle;
+extern crate rustc_span;
+
+use clippy_utils::diagnostics::span_lint;
+use if_chain::if_chain;
+use rustc_hir::{
+    intravisit::{walk_expr, FnKind, Visitor},
+    Body, Expr, ExprKind, FnDecl,
+};
+use rustc_lint::{LateContext, LateLintPass};
+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";
+
+dylint_linting::impl_late_lint! {
+    pub DYNAMIC_INSTANCE_STORAGE,
+    Warn,
+    LINT_MESSAGE,
+    DynamicInstanceStorage,
+    {
+        name: "Dynamic Instance Storage Analyzer",
+        long_message: "Detects potential misuse of instance storage that could lead to unnecessary growth or storage-related vulnerabilities.",
+        severity: "Warning",
+        help: "https://coinfabrik.github.io/scout-soroban/docs/detectors/dynamic-instance-storage",
+        vulnerability_class: "Resource Management",
+    }
+}
+
+#[derive(Default)]
+struct DynamicInstanceStorage;
+
+impl<'tcx> LateLintPass<'tcx> for DynamicInstanceStorage {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        _: FnKind<'tcx>,
+        _: &'tcx FnDecl<'tcx>,
+        body: &'tcx Body<'tcx>,
+        span: Span,
+        _: LocalDefId,
+    ) {
+        if span.from_expansion() {
+            return;
+        }
+
+        let mut storage_warn_visitor = DynamicInstanceStorageVisitor { cx };
+        storage_warn_visitor.visit_body(body);
+    }
+}
+
+struct DynamicInstanceStorageVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for DynamicInstanceStorageVisitor<'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
+            if let Some(receiver_ty) = get_node_type_opt(self.cx, &receiver.hir_id);
+            if is_soroban_storage(self.cx, receiver_ty, SorobanStorageType::Instance);
+            // 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)
+            }
+        }
+
+        walk_expr(self, expr)
+    }
+}
+
+fn is_dynamic_type(cx: &LateContext, ty: &Ty) -> bool {
+    match ty.kind() {
+        TyKind::Str => true,
+        TyKind::Slice(_) => true,
+        TyKind::Dynamic(..) => true,
+        TyKind::Array(element_ty, _) => is_dynamic_type(cx, element_ty),
+        TyKind::Adt(adt_def, _) => {
+            let type_name = cx.tcx.item_name(adt_def.did());
+            matches!(type_name.as_str(), "Vec" | "String" | "Map" | "LinkedList")
+        }
+        TyKind::RawPtr(ty, _) => is_dynamic_type(cx, ty),
+        TyKind::Ref(_, ty, _) => is_dynamic_type(cx, ty),
+        TyKind::Tuple(substs) => substs.iter().any(|ty| is_dynamic_type(cx, &ty)),
+        _ => false,
+    }
+}

scripts/run-tests.py

@@ -34,10 +34,10 @@ def run_tests(detector):
 
 def run_unit_tests(root):
     start_time = time.time()
-    returncode, stdout, _ = run_subprocess(["cargo", "test", "--all-features"], root)
+    returncode, _, stderr = run_subprocess(["cargo", "test", "--all-features"], root)
     print_results(
         returncode,
-        stdout,
+        stderr,
         "unit-test",
         root,
         time.time() - start_time,
@@ -48,6 +48,8 @@ def run_unit_tests(root):
 def run_integration_tests(detector, root):
     start_time = time.time()
 
+    detectors_path = os.path.join(os.getcwd(), "detectors")
+
     returncode, stdout, _ = run_subprocess(
         [
             "cargo",
@@ -56,7 +58,7 @@ def run_integration_tests(detector, root):
             detector,
             "--metadata",
             "--local-detectors",
-            os.path.join(os.getcwd(), "detectors"),
+            detectors_path,
         ],
         root,
     )
@@ -70,7 +72,7 @@ def run_integration_tests(detector, root):
     detector_metadata = parse_json_from_string(stdout)
 
     if not isinstance(detector_metadata, dict):
-        print("Failed to extract JSON:", detector_metadata)
+        print("Failed to extract JSON:\n", detector_metadata)
         return True
 
     detector_key = detector.replace("-", "_")

scripts/utils.py

@@ -37,7 +37,7 @@ def parse_json_from_string(console_output):
         except json.JSONDecodeError:
             return "Extracted string is not valid JSON"
     else:
-        return "No JSON found in the console output"
+        return console_output
 
 
 def run_subprocess(command: list, cwd: str):

test-cases/dynamic-instance-storage/Cargo.toml

@@ -0,0 +1,21 @@
+[workspace]
+exclude = [".cargo", "target"]
+members = ["dynamic-instance-storage-*/*"]
+resolver = "2"
+
+[workspace.dependencies]
+soroban-sdk = { version = "=21.4.0" }
+
+[profile.release]
+codegen-units = 1
+debug = 0
+debug-assertions = false
+lto = true
+opt-level = "z"
+overflow-checks = true
+panic = "abort"
+strip = "symbols"
+
+[profile.release-with-logs]
+debug-assertions = true
+inherits = "release"

test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+edition = "2021"
+name = "dynamic-instance-storage-remediated-1"
+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"]

test-cases/dynamic-instance-storage/dynamic-instance-storage-1/remediated-example/src/lib.rs

@@ -0,0 +1,43 @@
+#![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<i32>) {
+        e.storage()
+            .persistent()
+            .set(&Symbol::new(&e, "vector_data"), &data);
+    }
+
+    pub fn get_vector(e: Env) -> Vec<i32> {
+        e.storage()
+            .persistent()
+            .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);
+    }
+}

test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+edition = "2021"
+name = "dynamic-instance-storage-vulnerable-1"
+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"]

test-cases/dynamic-instance-storage/dynamic-instance-storage-1/vulnerable-example/src/lib.rs

@@ -0,0 +1,43 @@
+#![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<i32>) {
+        e.storage()
+            .instance()
+            .set(&Symbol::new(&e, "vector_data"), &data);
+    }
+
+    pub fn get_vector(e: Env) -> Vec<i32> {
+        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);
+    }
+}

test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+edition = "2021"
+name = "dynamic-instance-storage-remediated-2"
+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"]

test-cases/dynamic-instance-storage/dynamic-instance-storage-2/remediated-example/src/lib.rs

@@ -0,0 +1,43 @@
+#![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);
+    }
+}

test-cases/dynamic-instance-storage/dynamic-instance-storage-2/vulnerable-example/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+edition = "2021"
+name = "dynamic-instance-storage-vulnerable-2"
+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"]