From af0ea0f176e66d41005317713e87b45f3796c44e Mon Sep 17 00:00:00 2001 From: Matias Cabello Date: Thu, 21 Mar 2024 08:55:29 -0300 Subject: [PATCH 1/4] updated install and run commands in readme file --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 475cb80c..4938b1a4 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ cargo +nightly install cargo-dylint dylint-link Afterwards, install Scout with the following command: ```bash -cargo +nightly install cargo-scout-audit-soroban +cargo install cargo-scout-audit ``` ### CLI @@ -31,7 +31,7 @@ cargo +nightly install cargo-scout-audit-soroban To run Scout on your project, navigate to its root directory and execute the following command: ```bash -cargo scout-audit-soroban +cargo scout-audit ``` ### VSCode Extension From 41edebbe9849a1a7d0ba8859fc2647dc3ffd08d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Garc=C3=ADa=20Crosta?= Date: Thu, 21 Mar 2024 10:09:49 -0300 Subject: [PATCH 2/4] Detectors update (#91) * updated detectors por favor, actualizar los mensajes!!! * Update detectors dependencies * Update unused-return-enum detector and internal dependency * Update clippy-utils dependency --------- Co-authored-by: Facundo Lerena --- detectors/Cargo.toml | 6 ++-- detectors/avoid-core-mem-forget/src/lib.rs | 4 +-- detectors/avoid-panic-error/src/lib.rs | 6 ++-- detectors/avoid-unsafe-block/src/lib.rs | 4 +-- detectors/divide-before-multiply/src/lib.rs | 34 +++++++++---------- detectors/dos-unbounded-operation/src/lib.rs | 5 ++- .../insufficiently-random-values/src/lib.rs | 4 +-- detectors/overflow-check/src/lib.rs | 4 +-- detectors/rust-toolchain | 2 +- detectors/set-contract-storage/src/lib.rs | 4 +-- detectors/soroban-version/src/lib.rs | 4 +-- .../src/lib.rs | 31 ++++++++--------- detectors/unsafe-expect/src/lib.rs | 4 +-- detectors/unsafe-unwrap/src/lib.rs | 4 +-- detectors/unused-return-enum/src/lib.rs | 4 +-- scout-audit-clippy-utils/rust-toolchain | 2 +- scout-audit-internal/Cargo.toml | 2 +- scout-audit-internal/rust-toolchain | 2 +- 18 files changed, 60 insertions(+), 66 deletions(-) diff --git a/detectors/Cargo.toml b/detectors/Cargo.toml index 3b4c5494..0f641ca6 100644 --- a/detectors/Cargo.toml +++ b/detectors/Cargo.toml @@ -4,9 +4,9 @@ exclude = [".cargo", "target"] resolver = "2" [workspace.dependencies] -dylint_linting = "2.1.5" +dylint_linting = { version = "=2.6.1", package = "scout-audit-dylint-linting"} dylint_testing = "2.1.5" if_chain = "1.0.2" -scout-audit-clippy-utils = { version = "=0.2.1", path = "../scout-audit-clippy-utils" } -scout-audit-internal = { path = "../scout-audit-internal", features = ["detector", "lint_helper"] } +scout-audit-clippy-utils = { version = "=0.2.3" } +scout-audit-internal = { version="=0.2.4", features = ["detector", "lint_helper"] } diff --git a/detectors/avoid-core-mem-forget/src/lib.rs b/detectors/avoid-core-mem-forget/src/lib.rs index fa421c5e..5016bb67 100644 --- a/detectors/avoid-core-mem-forget/src/lib.rs +++ b/detectors/avoid-core-mem-forget/src/lib.rs @@ -8,7 +8,7 @@ use if_chain::if_chain; use rustc_ast::{Expr, ExprKind, Item, NodeId}; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_span::sym; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::impl_pre_expansion_lint! { /// ### What it does @@ -43,7 +43,7 @@ dylint_linting::impl_pre_expansion_lint! { pub AVOID_CORE_MEM_FORGET, Warn, - Detector::AvoidCoreMemForget.get_lint_message(), + "", AvoidCoreMemForget::default() } diff --git a/detectors/avoid-panic-error/src/lib.rs b/detectors/avoid-panic-error/src/lib.rs index b87812f8..a413521c 100644 --- a/detectors/avoid-panic-error/src/lib.rs +++ b/detectors/avoid-panic-error/src/lib.rs @@ -14,7 +14,7 @@ use rustc_ast::{ use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_span::{sym, Span}; use scout_audit_clippy_utils::sym; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::impl_pre_expansion_lint! { /// ### What it does @@ -57,7 +57,7 @@ dylint_linting::impl_pre_expansion_lint! { /// ``` pub AVOID_PANIC_ERROR, Warn, - Detector::AvoidPanicError.get_lint_message(), + "", AvoidPanicError::default() } @@ -182,6 +182,6 @@ fn capitalize_err_msg(s: &str) -> String { fn is_test_token_present(token_stream: &TokenStream) -> bool { token_stream.trees().any(|tree| match tree { TokenTree::Token(token, _) => token.is_ident_named(sym::test), - TokenTree::Delimited(_, _, token_stream) => is_test_token_present(token_stream), + TokenTree::Delimited(_, _, _, token_stream) => is_test_token_present(token_stream), }) } diff --git a/detectors/avoid-unsafe-block/src/lib.rs b/detectors/avoid-unsafe-block/src/lib.rs index 92c26a2a..89d02f1c 100644 --- a/detectors/avoid-unsafe-block/src/lib.rs +++ b/detectors/avoid-unsafe-block/src/lib.rs @@ -10,7 +10,7 @@ use rustc_hir::{ }; use rustc_lint::LateLintPass; use rustc_span::Span; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { /// ### What it does @@ -50,7 +50,7 @@ dylint_linting::declare_late_lint! { /// ``` pub AVOID_UNSAFE_BLOCK, Warn, - Detector::AvoidUnsafeBlock.get_lint_message() + "" } impl<'tcx> LateLintPass<'tcx> for AvoidUnsafeBlock { diff --git a/detectors/divide-before-multiply/src/lib.rs b/detectors/divide-before-multiply/src/lib.rs index 8c2b93a2..257843d2 100644 --- a/detectors/divide-before-multiply/src/lib.rs +++ b/detectors/divide-before-multiply/src/lib.rs @@ -20,7 +20,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::TyKind; use rustc_span::def_id::DefId; use rustc_span::Span; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { /// ### What it does @@ -45,7 +45,7 @@ dylint_linting::declare_late_lint! { /// ``` pub DIVIDE_BEFORE_MULTIPLY, Warn, - Detector::DivideBeforeMultiply.get_lint_message() + "" } fn get_divisions_inside_expr(expr: &Expr<'_>) -> Vec { @@ -298,23 +298,21 @@ fn navigate_trough_basicblocks<'tcx>( spans, ); } - TerminatorKind::InlineAsm { destination, .. } => { - if let Option::Some(dest) = destination { - navigate_trough_basicblocks( - *dest, - bbs, - def_ids, - tainted_places, - visited_bbs, - spans, - ); - } + TerminatorKind::InlineAsm { + destination: Some(dest), + .. + } => { + navigate_trough_basicblocks( + *dest, + bbs, + def_ids, + tainted_places, + visited_bbs, + spans, + ); } - TerminatorKind::GeneratorDrop - | TerminatorKind::UnwindResume - | TerminatorKind::UnwindTerminate(_) - | TerminatorKind::Return - | TerminatorKind::Unreachable => {} + + _ => {} } } } diff --git a/detectors/dos-unbounded-operation/src/lib.rs b/detectors/dos-unbounded-operation/src/lib.rs index 2d328eea..733535a6 100644 --- a/detectors/dos-unbounded-operation/src/lib.rs +++ b/detectors/dos-unbounded-operation/src/lib.rs @@ -14,7 +14,7 @@ use rustc_hir::{ }; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::{def_id::LocalDefId, Span}; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint!( pub DOS_UNBOUNDED_OPERATION, @@ -92,13 +92,12 @@ impl<'tcx> Visitor<'tcx> for ForLoopVisitor { if let ExprKind::Call(call_func, call_args) = match_expr.kind; // Check the function call if let ExprKind::Path(qpath) = &call_func.kind; - if let QPath::LangItem(LangItem::IntoIterIntoIter, _, _) = qpath; + if let QPath::LangItem(LangItem::IntoIterIntoIter, _) = qpath; // Check if a Range is used if let ExprKind::Struct(struct_lang_item, struct_expr, _) = call_args.first().unwrap().kind; if let QPath::LangItem( LangItem::Range | LangItem::RangeInclusiveStruct | LangItem::RangeInclusiveNew, _, - _, ) = struct_lang_item; // Get the start and end of the range if let Some(start_expr) = struct_expr.first(); diff --git a/detectors/insufficiently-random-values/src/lib.rs b/detectors/insufficiently-random-values/src/lib.rs index 5841f123..20c2e0a1 100644 --- a/detectors/insufficiently-random-values/src/lib.rs +++ b/detectors/insufficiently-random-values/src/lib.rs @@ -5,7 +5,7 @@ extern crate rustc_hir; use if_chain::if_chain; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { /// ### What it does @@ -21,7 +21,7 @@ dylint_linting::declare_late_lint! { /// pub INSUFFICIENTLY_RANDOM_VALUES, Warn, - Detector::InsufficientlyRandomValues.get_lint_message() + "" } impl<'tcx> LateLintPass<'tcx> for InsufficientlyRandomValues { diff --git a/detectors/overflow-check/src/lib.rs b/detectors/overflow-check/src/lib.rs index 0af11a96..cccc7081 100644 --- a/detectors/overflow-check/src/lib.rs +++ b/detectors/overflow-check/src/lib.rs @@ -7,7 +7,7 @@ extern crate rustc_span; use std::fs; use rustc_lint::EarlyLintPass; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_early_lint! { /// ### What it does @@ -19,7 +19,7 @@ dylint_linting::declare_early_lint! { /// wants explicitly checked, wrapping or saturating arithmetic. pub OVERFLOW_CHECK, Warn, - Detector::OverflowCheck.get_lint_message() + "" } impl EarlyLintPass for OverflowCheck { diff --git a/detectors/rust-toolchain b/detectors/rust-toolchain index 8811e92c..bcb80559 100644 --- a/detectors/rust-toolchain +++ b/detectors/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-09-29" +channel = "nightly-2023-12-16" components = ["llvm-tools-preview", "rustc-dev"] diff --git a/detectors/set-contract-storage/src/lib.rs b/detectors/set-contract-storage/src/lib.rs index 14606d8b..c8daf266 100644 --- a/detectors/set-contract-storage/src/lib.rs +++ b/detectors/set-contract-storage/src/lib.rs @@ -11,7 +11,7 @@ use rustc_hir::{Body, FnDecl}; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::Span; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { /// ### What it does @@ -38,7 +38,7 @@ dylint_linting::declare_late_lint! { /// ``` pub SET_STORAGE_WARN, Warn, - Detector::SetContractStorage.get_lint_message() + "" } impl<'tcx> LateLintPass<'tcx> for SetStorageWarn { diff --git a/detectors/soroban-version/src/lib.rs b/detectors/soroban-version/src/lib.rs index f4ba013e..240eea77 100644 --- a/detectors/soroban-version/src/lib.rs +++ b/detectors/soroban-version/src/lib.rs @@ -8,7 +8,7 @@ use std::{io::Error, process::Command}; use rustc_ast::Crate; use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; use semver::Version; use serde_json::Value; @@ -20,7 +20,7 @@ dylint_linting::declare_early_lint! { /// Using an outdated version of soroban could lead to security vulnerabilities, bugs, and other issues. pub CHECK_SOROBAN_VERSION, Warn, - Detector::SorobanVersion.get_lint_message() + "" } impl EarlyLintPass for CheckSorobanVersion { diff --git a/detectors/unprotected-update-current-contract-wasm/src/lib.rs b/detectors/unprotected-update-current-contract-wasm/src/lib.rs index bb5b5417..76a8bfd6 100644 --- a/detectors/unprotected-update-current-contract-wasm/src/lib.rs +++ b/detectors/unprotected-update-current-contract-wasm/src/lib.rs @@ -17,12 +17,12 @@ use rustc_middle::mir::{BasicBlock, BasicBlocks, Const, Operand, TerminatorKind} use rustc_middle::ty::TyKind; use rustc_span::def_id::DefId; use rustc_span::Span; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::impl_late_lint! { pub UNPROTECTED_UPDATE_CURRENT_CONTRACT_WASM, Warn, - Detector::UnprotectedUpdateCurrentContractWasm.get_lint_message(), + "", UnprotectedUpdateCurrentContractWasm::default() } @@ -179,22 +179,19 @@ impl<'tcx> LateLintPass<'tcx> for UnprotectedUpdateCurrentContractWasm { visited, )); } - TerminatorKind::InlineAsm { destination, .. } => { - if let Some(udestination) = destination { - ret_vec.append(&mut navigate_trough_basicblocks( - bbs, - *udestination, - checked, - uuf_storage, - visited, - )); - } + TerminatorKind::InlineAsm { + destination: Some(udestination), + .. + } => { + ret_vec.append(&mut navigate_trough_basicblocks( + bbs, + *udestination, + checked, + uuf_storage, + visited, + )); } - TerminatorKind::Return - | TerminatorKind::Unreachable - | TerminatorKind::GeneratorDrop - | TerminatorKind::UnwindResume - | TerminatorKind::UnwindTerminate(_) => {} + _ => {} } ret_vec } diff --git a/detectors/unsafe-expect/src/lib.rs b/detectors/unsafe-expect/src/lib.rs index 9b20182e..329c6292 100644 --- a/detectors/unsafe-expect/src/lib.rs +++ b/detectors/unsafe-expect/src/lib.rs @@ -10,7 +10,7 @@ use rustc_hir::{ }; use rustc_lint::LateLintPass; use rustc_span::{Span, Symbol}; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { /// ### What it does @@ -45,7 +45,7 @@ dylint_linting::declare_late_lint! { /// ``` pub UNSAFE_EXPECT, Warn, - Detector::UnsafeExpect.get_lint_message() + "" } impl<'tcx> LateLintPass<'tcx> for UnsafeExpect { diff --git a/detectors/unsafe-unwrap/src/lib.rs b/detectors/unsafe-unwrap/src/lib.rs index 08849cc8..4ecc650d 100644 --- a/detectors/unsafe-unwrap/src/lib.rs +++ b/detectors/unsafe-unwrap/src/lib.rs @@ -10,7 +10,7 @@ use rustc_hir::{ }; use rustc_lint::LateLintPass; use rustc_span::{Span, Symbol}; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { /// ### What it does @@ -45,7 +45,7 @@ dylint_linting::declare_late_lint! { /// ``` pub UNSAFE_UNWRAP, Warn, - Detector::UnsafeUnwrap.get_lint_message() + "" } impl<'tcx> LateLintPass<'tcx> for UnsafeUnwrap { diff --git a/detectors/unused-return-enum/src/lib.rs b/detectors/unused-return-enum/src/lib.rs index 458bad70..53747b6b 100644 --- a/detectors/unused-return-enum/src/lib.rs +++ b/detectors/unused-return-enum/src/lib.rs @@ -8,12 +8,12 @@ use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnRetTy, MatchSource, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::{def_id::LocalDefId, sym, Span}; -use scout_audit_internal::Detector; +use scout_audit_internal::{DetectorImpl, SorobanDetector as Detector}; dylint_linting::declare_late_lint! { pub UNUSED_RETURN_ENUM, Warn, - Detector::UnusedReturnEnum.get_lint_message() + "" } #[derive(Debug)] diff --git a/scout-audit-clippy-utils/rust-toolchain b/scout-audit-clippy-utils/rust-toolchain index 2628bc35..d575da6d 100644 --- a/scout-audit-clippy-utils/rust-toolchain +++ b/scout-audit-clippy-utils/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-09-29" +channel = "nightly-2023-12-16" components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/scout-audit-internal/Cargo.toml b/scout-audit-internal/Cargo.toml index f98702ca..28bae192 100644 --- a/scout-audit-internal/Cargo.toml +++ b/scout-audit-internal/Cargo.toml @@ -28,4 +28,4 @@ lint_helper = [ [dependencies] strum = { version = "0.25", features = ["derive"], optional = true } serde_json = { version = "1.0", optional = true } -scout-audit-clippy-utils = { version = "=0.2.1", path = "../scout-audit-clippy-utils", optional = true } +scout-audit-clippy-utils = { version = "=0.2.3", optional = true } diff --git a/scout-audit-internal/rust-toolchain b/scout-audit-internal/rust-toolchain index 8811e92c..bcb80559 100644 --- a/scout-audit-internal/rust-toolchain +++ b/scout-audit-internal/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-09-29" +channel = "nightly-2023-12-16" components = ["llvm-tools-preview", "rustc-dev"] From 4e261326eb07867b6e123fa5737cc59e70795a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Garc=C3=ADa=20Crosta?= Date: Mon, 25 Mar 2024 09:48:47 -0300 Subject: [PATCH 3/4] 61 unprotected mapping operation (#87) * Add unprotected-mapping operation detector * Add test cases * Edit detector * Edit soroban-sdk version * Allow unordered mapping * Remove println * Add unprotected mapping operation to detectors table. * Add unprotected mapping operation vulnerability class * Add unprotected mapping operation detector documentation * Update test case link for unprotected mapping operation * Update detector to check type and key of mapping --------- Co-authored-by: Arturo Beccar-Varela <107512933+arturoBeccar@users.noreply.github.com> --- .github/workflows/test-detectors.yml | 1 + README.md | 3 +- .../unprotected-mapping-operation/Cargo.toml | 20 ++ .../unprotected-mapping-operation/README.md | 60 +++++ .../unprotected-mapping-operation/src/lib.rs | 208 ++++++++++++++++++ scout-audit-internal/src/detector.rs | 4 +- .../src/detector/lint_message.rs | 2 + test-cases/README.md | 5 + .../remediated-example/Cargo.toml | 31 +++ .../remediated-example/src/lib.rs | 71 ++++++ .../vulnerable-example/Cargo.toml | 31 +++ .../vulnerable-example/src/lib.rs | 66 ++++++ 12 files changed, 500 insertions(+), 2 deletions(-) create mode 100644 detectors/unprotected-mapping-operation/Cargo.toml create mode 100644 detectors/unprotected-mapping-operation/README.md create mode 100644 detectors/unprotected-mapping-operation/src/lib.rs create mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml create mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs create mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml create mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index 3c17adc4..d99cd109 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -105,6 +105,7 @@ jobs: "insufficiently-random-values", "overflow-check", "set-contract-storage", + "unprotected-mapping-operation", "unprotected-update-current-contract-wasm", "unsafe-expect", "unsafe-unwrap", diff --git a/README.md b/README.md index 4938b1a4..77c71475 100644 --- a/README.md +++ b/README.md @@ -50,12 +50,13 @@ Install Scout from the Marketplace within the Extensions tab of Visual Studio Co | [overflow-check](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/overflow-check) | An arithmetic operation overflows or underflows the available memory allocated to the variable. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/overflow-check/overflow-check-1)| Critical | | [insufficiently-random-values](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/insufficiently-random-values) | Avoid using block attributes for random number generation to prevent manipulation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/insufficiently-random-values/insufficiently-random-values-1)| Critical | | [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical | -| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhancement | +| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhancement | | [set-contract-storage](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/set-contract-storage) | Insufficient access control on `env.storage()` method. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-2), [3](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-3) | Critical | | [avoid-panic-error](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-panic-error) | Code panics on error instead of using descriptive enum.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-panic-error/avoid-panic-error-1) | Enhancement | | [avoid-unsafe-block](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-unsafe-block) | Using unsafe blocks in risks code safety and reliability.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-unsafe-block/avoid-unsafe-block-1) | Critical | | [dos-unbounded-operation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dos-unbounded-operation) | DoS due to unbounded operation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-2), [3](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-3) | Medium | | [soroban-version](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version) | Using a pinned version of Soroban can be dangerous, as it may have bugs or security issues. Use the latest version available. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1) | Enhancement | +| [unprotected-mapping-operation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-mapping-opereation) | Modifying mappings with an arbitrary key given by the user could lead to unintented modifications of critical data, modifying data belonging to other users, causing denial of service, unathorized access, and other potential issues. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-mapping-opereation/unprotected-mapping-opereation-1) | Critical | | [unused-return-enum](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unused-return-enum) | Return enum from a function is not completely used. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unused-return-enum/unused-return-enum-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unused-return-enum/unused-return-enum-2) | Minor | ## CLI Options diff --git a/detectors/unprotected-mapping-operation/Cargo.toml b/detectors/unprotected-mapping-operation/Cargo.toml new file mode 100644 index 00000000..a3841f94 --- /dev/null +++ b/detectors/unprotected-mapping-operation/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "unprotected-mapping-operation" +version = "0.1.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +dylint_linting = { workspace = true } +if_chain = { workspace = true } + +scout-audit-internal = { workspace = true } + +[dev-dependencies] +dylint_testing = { workspace = true } + + +[package.metadata.rust-analyzer] +rustc_private = true diff --git a/detectors/unprotected-mapping-operation/README.md b/detectors/unprotected-mapping-operation/README.md new file mode 100644 index 00000000..e06f9710 --- /dev/null +++ b/detectors/unprotected-mapping-operation/README.md @@ -0,0 +1,60 @@ +# Unprotected Mapping Operation + +### What it does + +It warns you if a mapping operation (`insert`, `take`, `remove`) function is called with a user-given `key` field of the type `AccountId`. + +### Why is this bad? + +Modifying mappings with an arbitrary key given by users can be a significant vulnerability for several reasons: + +- Unintended Modifications: Allowing users to provide arbitrary keys can lead to unintended modifications of critical data within the smart contract. If the input validation and sanitation are not done properly, users may be able to manipulate the data in ways that were not intended by the contract's author. + +- Data Corruption: Malicious users could intentionally provide keys that result in the corruption or manipulation of important data stored in the mapping. This could lead to incorrect calculations, unauthorized access, or other undesirable outcomes. + +- Denial-of-Service (DoS) Attacks: If users can set arbitrary keys, they may be able to create mappings with a large number of entries, potentially causing the contract to exceed its gas limit. This could lead to denial-of-service attacks, making the contract unusable for other users. + +### Known problems + +### Example + +```rust + pub fn set_balance(env: Env, address: Address, balance: i128) -> State { + // Get the current state. + let mut state = Self::get_state(env.clone()); + + // Set the new account to have total supply if it doesn't exist. + if !state.balances.contains_key(address.clone()) { + state.balances.set(address, balance); + // Save the state. + env.storage().persistent().set(&STATE, &state); + } + + state + } +``` + +Use instead: + +```rust + pub fn set_balance(env: Env, address: Address, balance: i128) -> State { + // Authenticate user + address.require_auth(); + + // Get the current state. + let mut state = Self::get_state(env.clone()); + + // Set the new account to have total supply if it doesn't exist. + if !state.balances.contains_key(address.clone()) { + state.balances.set(address, balance); + // Save the state. + env.storage().persistent().set(&STATE, &state); + } + + state + } +``` + +### Implementation + +The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-mapping-operation). diff --git a/detectors/unprotected-mapping-operation/src/lib.rs b/detectors/unprotected-mapping-operation/src/lib.rs new file mode 100644 index 00000000..078b6a30 --- /dev/null +++ b/detectors/unprotected-mapping-operation/src/lib.rs @@ -0,0 +1,208 @@ +#![feature(rustc_private)] + +extern crate rustc_ast; +extern crate rustc_hir; +extern crate rustc_middle; +extern crate rustc_span; + +use if_chain::if_chain; +use rustc_hir::{ + def::Res, + intravisit::{walk_body, walk_expr, Visitor}, + Expr, ExprKind, HirId, Param, PatKind, QPath, StmtKind, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{Ty, TyKind}; +use rustc_span::{Span, Symbol}; +use scout_audit_internal::Detector; + +dylint_linting::declare_late_lint! { + pub UNPROTECTED_MAPPING_OPERATION, + Warn, + Detector::UnprotectedMappingOperation.get_lint_message() +} + +struct AuthStatus { + authed: bool, +} + +struct UnauthorizedAddress { + span: Span, + name: String, +} + +struct UnprotectedMappingOperationFinder<'tcx, 'tcx_ref> { + cx: &'tcx_ref LateContext<'tcx>, + linked_addresses: Vec<(AuthStatus, Vec)>, + unauthorized_span: Vec, +} + +const SOROBAN_MAP: &str = "soroban_sdk::Map"; +const SOROBAN_ADDRESS: &str = "soroban_sdk::Address"; + +impl<'tcx> LateLintPass<'tcx> for UnprotectedMappingOperation { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: rustc_hir::intravisit::FnKind<'tcx>, + _: &'tcx rustc_hir::FnDecl<'tcx>, + body: &'tcx rustc_hir::Body<'tcx>, + _: Span, + _: rustc_span::def_id::LocalDefId, + ) { + let mut visitor = UnprotectedMappingOperationFinder { + cx, + linked_addresses: Vec::new(), + unauthorized_span: Vec::new(), + }; + + visitor.parse_body_params(body.params); + + walk_body(&mut visitor, body); + + visitor + .unauthorized_span + .iter() + .for_each(|unauthorized_address| { + Detector::UnprotectedMappingOperation.span_lint_and_help( + cx, + UNPROTECTED_MAPPING_OPERATION, + unauthorized_address.span, + &format!( + "Address not authorized, please use `{}.require_auth();` prior to the mapping operation", + unauthorized_address.name + ), + ); + }); + } +} + +impl<'tcx> Visitor<'tcx> for UnprotectedMappingOperationFinder<'tcx, '_> { + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + if let ExprKind::Block(block, _) = &expr.kind { + block.stmts.iter().for_each(|stmt| { + if_chain! { + if let StmtKind::Local(local) = &stmt.kind; + if self.get_node_type(local.hir_id).to_string() == SOROBAN_ADDRESS; + if let PatKind::Binding(_, target_hir_id, _, _) = &local.pat.kind; + if let Some(init) = &local.init; + let source_hir_id = self.get_expr_hir_id(init); + if let Some(source_hir_id) = source_hir_id; + then { + // Insert the new address into the linked_addresses + self.insert_new_address(source_hir_id, *target_hir_id); + } + } + }) + } + + if let ExprKind::MethodCall(method_path, method_expr, method_args, _) = &expr.kind { + // Get the method expression type and check if it's a map with address + let method_expr_type = self.get_node_type(method_expr.hir_id); + + if_chain! { + if let ExprKind::Field(_, _) = &method_expr.kind; + if let TyKind::Adt(adt_def, args) = method_expr_type.kind(); + if self.cx.tcx.def_path_str(adt_def.did()) == SOROBAN_MAP; + if let Some(first_arg) = args.first(); + if first_arg.to_string() == SOROBAN_ADDRESS; + then { + // Iterate through the method arguments and check if any of them is an address and not authed + method_args.iter().for_each(|arg| { + if_chain! { + if let Some(id) = self.get_expr_hir_id(arg); + if self.get_node_type(id).to_string().contains(SOROBAN_ADDRESS); + then { + // Obtain the linked_addresses record in wich the address id is contained + let linked_address = self.get_linked_address(id); + + // If the address does not exist, of if it does but the AuthStatus is false, then we need to add it to the unauthorized_span + if linked_address.is_none() || !linked_address.unwrap().0.authed { + self.unauthorized_span.push(UnauthorizedAddress { + span: expr.span, + name: self.cx.tcx.hir().name(id).to_string(), + }); + } + } + } + }); + } + } + + // Check if the method call is a require_auth call and if it is, then we need to update the AuthStatus + if_chain! { + if method_expr_type.to_string() == SOROBAN_ADDRESS; + if method_path.ident.name == Symbol::intern("require_auth"); + if let Some(id) = self.get_expr_hir_id(method_expr); + then { + self.auth_address(id) + } + } + } + + walk_expr(self, expr); + } +} + +impl<'tcx> UnprotectedMappingOperationFinder<'tcx, '_> { + fn parse_body_params(&mut self, params: &'tcx [Param<'_>]) { + params.iter().for_each(|param| { + if self.get_node_type(param.hir_id).to_string() == SOROBAN_ADDRESS { + self.linked_addresses + .push((AuthStatus { authed: false }, vec![param.pat.hir_id])); + } + }); + } + + fn get_node_type(&self, hir_id: HirId) -> Ty<'tcx> { + self.cx.typeck_results().node_type(hir_id) + } + + fn insert_new_address(&mut self, source_hir_id: HirId, target_hir_id: HirId) { + if let Some((_, linked_addresses)) = self + .linked_addresses + .iter_mut() + .find(|(_, addresses)| addresses.iter().any(|&id| id == source_hir_id)) + { + linked_addresses.push(target_hir_id); + } + } + + fn get_expr_hir_id(&self, expr: &Expr) -> Option { + let mut stack = vec![expr]; + + while let Some(current_expr) = stack.pop() { + match current_expr.kind { + ExprKind::MethodCall(_, call_expr, _, _) => stack.push(call_expr), + ExprKind::Path(QPath::Resolved(_, path)) => match path.res { + Res::Local(hir_id) => return Some(hir_id), + _ => continue, + }, + _ => continue, + } + } + + None + } + + fn get_linked_address(&self, id: HirId) -> Option<&(AuthStatus, Vec)> { + self.linked_addresses.iter().find(|(_, linked_addresses)| { + linked_addresses + .iter() + .any(|&address_hir_id| address_hir_id == id) + }) + } + + fn auth_address(&mut self, id: HirId) { + self.linked_addresses + .iter_mut() + .for_each(|(auth_status, linked_addresses)| { + if linked_addresses + .iter() + .any(|&address_hir_id| address_hir_id == id) + { + auth_status.authed = true; + } + }); + } +} diff --git a/scout-audit-internal/src/detector.rs b/scout-audit-internal/src/detector.rs index b6914bed..32bf721c 100644 --- a/scout-audit-internal/src/detector.rs +++ b/scout-audit-internal/src/detector.rs @@ -35,6 +35,7 @@ pub enum Detector { OverflowCheck, SetContractStorage, SorobanVersion, + UnprotectedMappingOperation, UnprotectedUpdateCurrentContractWasm, UnsafeExpect, UnsafeUnwrap, @@ -47,16 +48,17 @@ impl Detector { match self { Detector::AvoidCoreMemForget => AVOID_CORE_MEM_FORGET_LINT_MESSAGE, Detector::AvoidPanicError => AVOID_PANIC_ERROR_LINT_MESSAGE, + Detector::AvoidUnsafeBlock => AVOID_UNSAFE_BLOCK_LINT_MESSAGE, Detector::DivideBeforeMultiply => DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE, Detector::DosUnboundedOperation => DOS_UNBOUNDED_OPERATION_LINT_MESSAGE, Detector::InsufficientlyRandomValues => INSUFFICIENTLY_RANDOM_VALUES_LINT_MESSAGE, Detector::OverflowCheck => OVERFLOW_CHECK_LINT_MESSAGE, Detector::SetContractStorage => SET_CONTRACT_STORAGE_LINT_MESSAGE, Detector::SorobanVersion => SOROBAN_VERSION_LINT_MESSAGE, + Detector::UnprotectedMappingOperation => UNPROTECTED_MAPPING_OPERATION_LINT_MESSAGE, Detector::UnprotectedUpdateCurrentContractWasm => { UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE } - Detector::AvoidUnsafeBlock => AVOID_UNSAFE_BLOCK_LINT_MESSAGE, Detector::UnsafeExpect => UNSAFE_EXPECT_LINT_MESSAGE, Detector::UnsafeUnwrap => UNSAFE_UNWRAP_LINT_MESSAGE, Detector::UnusedReturnEnum => UNUSED_RETURN_ENUM_LINT_MESSAGE, diff --git a/scout-audit-internal/src/detector/lint_message.rs b/scout-audit-internal/src/detector/lint_message.rs index 4cf87b5c..01358a13 100644 --- a/scout-audit-internal/src/detector/lint_message.rs +++ b/scout-audit-internal/src/detector/lint_message.rs @@ -12,6 +12,8 @@ pub const DOS_UNBOUNDED_OPERATION_LINT_MESSAGE: &str = pub const OVERFLOW_CHECK_LINT_MESSAGE: &str = "Use `overflow-checks = true` in Cargo.toml profile"; pub const SET_CONTRACT_STORAGE_LINT_MESSAGE:&str = "Abitrary users should not have control over keys because it implies writing any value of left mapping, lazy variable, or the main struct of the contract located in position 0 of the storage"; pub const SOROBAN_VERSION_LINT_MESSAGE: &str = "Use the latest version of Soroban"; +pub const UNPROTECTED_MAPPING_OPERATION_LINT_MESSAGE: &str = + "This mapping operation is called without access control on a different key than the caller's address"; pub const UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE: &str = "This update_current_contract_wasm is called without access control"; pub const UNSAFE_EXPECT_LINT_MESSAGE: &str = "Unsafe usage of `expect`"; diff --git a/test-cases/README.md b/test-cases/README.md index 18afcac7..9b9c6127 100644 --- a/test-cases/README.md +++ b/test-cases/README.md @@ -192,6 +192,11 @@ Using a pinned version of Soroban can be dangerous, as it may have bugs or secur We classified this issue, a deviation from best practices which could have security implications, under the [Best practices](#vulnerability-categories) category and assigned it an Enhancement severity. +### Unprotected mapping operation + +Modifying mappings with an arbitrary key given by the user could lead to unintented modifications of critical data, modifying data belonging to other users, causing denial of service, unathorized access, and other potential issues. + +This vulnerability falls under the [Validations and error handling](#vulnerability-categories) category and has a Critical severity. ### Unused return enum `Rust` messages can return a `Result` `enum` with a custom error type. This is diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml new file mode 100644 index 00000000..5e367c2e --- /dev/null +++ b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml @@ -0,0 +1,31 @@ + +[package] +name = "unprotected-mapping-operation" +version = "0.1.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = "=20.0.0" + +[dev_dependencies] +soroban-sdk = { version = "=20.0.0", features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] + +[profile.release] +opt-level = "z" +overflow-checks = true +debug = 0 +strip = "symbols" +debug-assertions = false +panic = "abort" +codegen-units = 1 +lto = true + +[profile.release-with-logs] +inherits = "release" +debug-assertions = true diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs new file mode 100644 index 00000000..f3c98e50 --- /dev/null +++ b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs @@ -0,0 +1,71 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, contracttype, symbol_short, Address, Env, Map, Symbol}; + +#[contract] +pub struct UnprotectedMappingOperation; + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct State { + balances: Map, +} +const STATE: Symbol = symbol_short!("STATE"); + +#[contractimpl] +impl UnprotectedMappingOperation { + pub fn set_balance(env: Env, address: Address, balance: i128) -> State { + // Authenticate user + address.require_auth(); + + // Get the current state. + let mut state = Self::get_state(env.clone()); + + // Set the new account to have total supply if it doesn't exist. + if !state.balances.contains_key(address.clone()) { + state.balances.set(address, balance); + // Save the state. + env.storage().persistent().set(&STATE, &state); + } + + state + } + + /// Return the current state. + pub fn get_state(env: Env) -> State { + env.storage().persistent().get(&STATE).unwrap_or(State { + balances: Map::new(&env), + }) + } +} + +#[cfg(test)] +const TOTAL_SUPPLY: i128 = 200; + +#[cfg(test)] +mod tests { + + use soroban_sdk::Env; + + use crate::{UnprotectedMappingOperation, UnprotectedMappingOperationClient, TOTAL_SUPPLY}; + + #[test] + fn balance_of_works() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, UnprotectedMappingOperation); + let client = UnprotectedMappingOperationClient::new(&env, &contract_id); + + // When + let state = client + .mock_all_auths() + .set_balance(&contract_id, &TOTAL_SUPPLY); + + // Then + let balance = state + .balances + .get(contract_id) + .expect("Contract should have a balance"); + assert_eq!(TOTAL_SUPPLY, balance); + } +} diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml new file mode 100644 index 00000000..5e367c2e --- /dev/null +++ b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml @@ -0,0 +1,31 @@ + +[package] +name = "unprotected-mapping-operation" +version = "0.1.0" +edition = "2021" + +[lib] +crate-type = ["cdylib"] + +[dependencies] +soroban-sdk = "=20.0.0" + +[dev_dependencies] +soroban-sdk = { version = "=20.0.0", features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] + +[profile.release] +opt-level = "z" +overflow-checks = true +debug = 0 +strip = "symbols" +debug-assertions = false +panic = "abort" +codegen-units = 1 +lto = true + +[profile.release-with-logs] +inherits = "release" +debug-assertions = true diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs new file mode 100644 index 00000000..34a88680 --- /dev/null +++ b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs @@ -0,0 +1,66 @@ +#![no_std] + +use soroban_sdk::{contract, contractimpl, contracttype, symbol_short, Address, Env, Map, Symbol}; + +#[contract] +pub struct UnprotectedMappingOperation; + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct State { + balances: Map, +} +const STATE: Symbol = symbol_short!("STATE"); + +#[contractimpl] +impl UnprotectedMappingOperation { + pub fn set_balance(env: Env, address: Address, balance: i128) -> State { + // Get the current state. + let mut state = Self::get_state(env.clone()); + + // Set the new account to have total supply if it doesn't exist. + if !state.balances.contains_key(address.clone()) { + state.balances.set(address, balance); + // Save the state. + env.storage().persistent().set(&STATE, &state); + } + + state + } + + /// Return the current state. + pub fn get_state(env: Env) -> State { + env.storage().persistent().get(&STATE).unwrap_or(State { + balances: Map::new(&env), + }) + } +} + +#[cfg(test)] +const TOTAL_SUPPLY: i128 = 200; + +#[cfg(test)] +mod tests { + + use soroban_sdk::Env; + + use crate::{UnprotectedMappingOperation, UnprotectedMappingOperationClient, TOTAL_SUPPLY}; + + #[test] + fn balance_of_works() { + // Given + let env = Env::default(); + let contract_id = env.register_contract(None, UnprotectedMappingOperation); + let client = UnprotectedMappingOperationClient::new(&env, &contract_id); + + // When + let state = client.set_balance(&contract_id, &TOTAL_SUPPLY); + + // Then + let balance = state + .balances + .get(contract_id) + .expect("Contract should have a balance"); + assert_eq!(TOTAL_SUPPLY, balance); + } +} From fb2485f4ee7f92caf7d58a86d90d46492471842a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Garc=C3=ADa=20Crosta?= Date: Mon, 25 Mar 2024 11:06:31 -0300 Subject: [PATCH 4/4] Revert "61 unprotected mapping operation (#87)" (#94) This reverts commit 4e261326eb07867b6e123fa5737cc59e70795a40. --- .github/workflows/test-detectors.yml | 1 - README.md | 3 +- .../unprotected-mapping-operation/Cargo.toml | 20 -- .../unprotected-mapping-operation/README.md | 60 ----- .../unprotected-mapping-operation/src/lib.rs | 208 ------------------ scout-audit-internal/src/detector.rs | 4 +- .../src/detector/lint_message.rs | 2 - test-cases/README.md | 5 - .../remediated-example/Cargo.toml | 31 --- .../remediated-example/src/lib.rs | 71 ------ .../vulnerable-example/Cargo.toml | 31 --- .../vulnerable-example/src/lib.rs | 66 ------ 12 files changed, 2 insertions(+), 500 deletions(-) delete mode 100644 detectors/unprotected-mapping-operation/Cargo.toml delete mode 100644 detectors/unprotected-mapping-operation/README.md delete mode 100644 detectors/unprotected-mapping-operation/src/lib.rs delete mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml delete mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs delete mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml delete mode 100644 test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs diff --git a/.github/workflows/test-detectors.yml b/.github/workflows/test-detectors.yml index d99cd109..3c17adc4 100644 --- a/.github/workflows/test-detectors.yml +++ b/.github/workflows/test-detectors.yml @@ -105,7 +105,6 @@ jobs: "insufficiently-random-values", "overflow-check", "set-contract-storage", - "unprotected-mapping-operation", "unprotected-update-current-contract-wasm", "unsafe-expect", "unsafe-unwrap", diff --git a/README.md b/README.md index 77c71475..4938b1a4 100644 --- a/README.md +++ b/README.md @@ -50,13 +50,12 @@ Install Scout from the Marketplace within the Extensions tab of Visual Studio Co | [overflow-check](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/overflow-check) | An arithmetic operation overflows or underflows the available memory allocated to the variable. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/overflow-check/overflow-check-1)| Critical | | [insufficiently-random-values](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/insufficiently-random-values) | Avoid using block attributes for random number generation to prevent manipulation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/insufficiently-random-values/insufficiently-random-values-1)| Critical | | [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical | -| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhancement | +| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhancement | | [set-contract-storage](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/set-contract-storage) | Insufficient access control on `env.storage()` method. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-2), [3](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/set-contract-storage/set-contract-storage-3) | Critical | | [avoid-panic-error](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-panic-error) | Code panics on error instead of using descriptive enum.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-panic-error/avoid-panic-error-1) | Enhancement | | [avoid-unsafe-block](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-unsafe-block) | Using unsafe blocks in risks code safety and reliability.| [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-unsafe-block/avoid-unsafe-block-1) | Critical | | [dos-unbounded-operation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/dos-unbounded-operation) | DoS due to unbounded operation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-2), [3](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/dos-unbounded-operation/dos-unbounded-operation-3) | Medium | | [soroban-version](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/soroban-version) | Using a pinned version of Soroban can be dangerous, as it may have bugs or security issues. Use the latest version available. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/soroban-version/soroban-version-1) | Enhancement | -| [unprotected-mapping-operation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-mapping-opereation) | Modifying mappings with an arbitrary key given by the user could lead to unintented modifications of critical data, modifying data belonging to other users, causing denial of service, unathorized access, and other potential issues. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-mapping-opereation/unprotected-mapping-opereation-1) | Critical | | [unused-return-enum](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unused-return-enum) | Return enum from a function is not completely used. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unused-return-enum/unused-return-enum-1), [2](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unused-return-enum/unused-return-enum-2) | Minor | ## CLI Options diff --git a/detectors/unprotected-mapping-operation/Cargo.toml b/detectors/unprotected-mapping-operation/Cargo.toml deleted file mode 100644 index a3841f94..00000000 --- a/detectors/unprotected-mapping-operation/Cargo.toml +++ /dev/null @@ -1,20 +0,0 @@ -[package] -name = "unprotected-mapping-operation" -version = "0.1.0" -edition = "2021" - -[lib] -crate-type = ["cdylib"] - -[dependencies] -dylint_linting = { workspace = true } -if_chain = { workspace = true } - -scout-audit-internal = { workspace = true } - -[dev-dependencies] -dylint_testing = { workspace = true } - - -[package.metadata.rust-analyzer] -rustc_private = true diff --git a/detectors/unprotected-mapping-operation/README.md b/detectors/unprotected-mapping-operation/README.md deleted file mode 100644 index e06f9710..00000000 --- a/detectors/unprotected-mapping-operation/README.md +++ /dev/null @@ -1,60 +0,0 @@ -# Unprotected Mapping Operation - -### What it does - -It warns you if a mapping operation (`insert`, `take`, `remove`) function is called with a user-given `key` field of the type `AccountId`. - -### Why is this bad? - -Modifying mappings with an arbitrary key given by users can be a significant vulnerability for several reasons: - -- Unintended Modifications: Allowing users to provide arbitrary keys can lead to unintended modifications of critical data within the smart contract. If the input validation and sanitation are not done properly, users may be able to manipulate the data in ways that were not intended by the contract's author. - -- Data Corruption: Malicious users could intentionally provide keys that result in the corruption or manipulation of important data stored in the mapping. This could lead to incorrect calculations, unauthorized access, or other undesirable outcomes. - -- Denial-of-Service (DoS) Attacks: If users can set arbitrary keys, they may be able to create mappings with a large number of entries, potentially causing the contract to exceed its gas limit. This could lead to denial-of-service attacks, making the contract unusable for other users. - -### Known problems - -### Example - -```rust - pub fn set_balance(env: Env, address: Address, balance: i128) -> State { - // Get the current state. - let mut state = Self::get_state(env.clone()); - - // Set the new account to have total supply if it doesn't exist. - if !state.balances.contains_key(address.clone()) { - state.balances.set(address, balance); - // Save the state. - env.storage().persistent().set(&STATE, &state); - } - - state - } -``` - -Use instead: - -```rust - pub fn set_balance(env: Env, address: Address, balance: i128) -> State { - // Authenticate user - address.require_auth(); - - // Get the current state. - let mut state = Self::get_state(env.clone()); - - // Set the new account to have total supply if it doesn't exist. - if !state.balances.contains_key(address.clone()) { - state.balances.set(address, balance); - // Save the state. - env.storage().persistent().set(&STATE, &state); - } - - state - } -``` - -### Implementation - -The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-mapping-operation). diff --git a/detectors/unprotected-mapping-operation/src/lib.rs b/detectors/unprotected-mapping-operation/src/lib.rs deleted file mode 100644 index 078b6a30..00000000 --- a/detectors/unprotected-mapping-operation/src/lib.rs +++ /dev/null @@ -1,208 +0,0 @@ -#![feature(rustc_private)] - -extern crate rustc_ast; -extern crate rustc_hir; -extern crate rustc_middle; -extern crate rustc_span; - -use if_chain::if_chain; -use rustc_hir::{ - def::Res, - intravisit::{walk_body, walk_expr, Visitor}, - Expr, ExprKind, HirId, Param, PatKind, QPath, StmtKind, -}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{Ty, TyKind}; -use rustc_span::{Span, Symbol}; -use scout_audit_internal::Detector; - -dylint_linting::declare_late_lint! { - pub UNPROTECTED_MAPPING_OPERATION, - Warn, - Detector::UnprotectedMappingOperation.get_lint_message() -} - -struct AuthStatus { - authed: bool, -} - -struct UnauthorizedAddress { - span: Span, - name: String, -} - -struct UnprotectedMappingOperationFinder<'tcx, 'tcx_ref> { - cx: &'tcx_ref LateContext<'tcx>, - linked_addresses: Vec<(AuthStatus, Vec)>, - unauthorized_span: Vec, -} - -const SOROBAN_MAP: &str = "soroban_sdk::Map"; -const SOROBAN_ADDRESS: &str = "soroban_sdk::Address"; - -impl<'tcx> LateLintPass<'tcx> for UnprotectedMappingOperation { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - _: rustc_hir::intravisit::FnKind<'tcx>, - _: &'tcx rustc_hir::FnDecl<'tcx>, - body: &'tcx rustc_hir::Body<'tcx>, - _: Span, - _: rustc_span::def_id::LocalDefId, - ) { - let mut visitor = UnprotectedMappingOperationFinder { - cx, - linked_addresses: Vec::new(), - unauthorized_span: Vec::new(), - }; - - visitor.parse_body_params(body.params); - - walk_body(&mut visitor, body); - - visitor - .unauthorized_span - .iter() - .for_each(|unauthorized_address| { - Detector::UnprotectedMappingOperation.span_lint_and_help( - cx, - UNPROTECTED_MAPPING_OPERATION, - unauthorized_address.span, - &format!( - "Address not authorized, please use `{}.require_auth();` prior to the mapping operation", - unauthorized_address.name - ), - ); - }); - } -} - -impl<'tcx> Visitor<'tcx> for UnprotectedMappingOperationFinder<'tcx, '_> { - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if let ExprKind::Block(block, _) = &expr.kind { - block.stmts.iter().for_each(|stmt| { - if_chain! { - if let StmtKind::Local(local) = &stmt.kind; - if self.get_node_type(local.hir_id).to_string() == SOROBAN_ADDRESS; - if let PatKind::Binding(_, target_hir_id, _, _) = &local.pat.kind; - if let Some(init) = &local.init; - let source_hir_id = self.get_expr_hir_id(init); - if let Some(source_hir_id) = source_hir_id; - then { - // Insert the new address into the linked_addresses - self.insert_new_address(source_hir_id, *target_hir_id); - } - } - }) - } - - if let ExprKind::MethodCall(method_path, method_expr, method_args, _) = &expr.kind { - // Get the method expression type and check if it's a map with address - let method_expr_type = self.get_node_type(method_expr.hir_id); - - if_chain! { - if let ExprKind::Field(_, _) = &method_expr.kind; - if let TyKind::Adt(adt_def, args) = method_expr_type.kind(); - if self.cx.tcx.def_path_str(adt_def.did()) == SOROBAN_MAP; - if let Some(first_arg) = args.first(); - if first_arg.to_string() == SOROBAN_ADDRESS; - then { - // Iterate through the method arguments and check if any of them is an address and not authed - method_args.iter().for_each(|arg| { - if_chain! { - if let Some(id) = self.get_expr_hir_id(arg); - if self.get_node_type(id).to_string().contains(SOROBAN_ADDRESS); - then { - // Obtain the linked_addresses record in wich the address id is contained - let linked_address = self.get_linked_address(id); - - // If the address does not exist, of if it does but the AuthStatus is false, then we need to add it to the unauthorized_span - if linked_address.is_none() || !linked_address.unwrap().0.authed { - self.unauthorized_span.push(UnauthorizedAddress { - span: expr.span, - name: self.cx.tcx.hir().name(id).to_string(), - }); - } - } - } - }); - } - } - - // Check if the method call is a require_auth call and if it is, then we need to update the AuthStatus - if_chain! { - if method_expr_type.to_string() == SOROBAN_ADDRESS; - if method_path.ident.name == Symbol::intern("require_auth"); - if let Some(id) = self.get_expr_hir_id(method_expr); - then { - self.auth_address(id) - } - } - } - - walk_expr(self, expr); - } -} - -impl<'tcx> UnprotectedMappingOperationFinder<'tcx, '_> { - fn parse_body_params(&mut self, params: &'tcx [Param<'_>]) { - params.iter().for_each(|param| { - if self.get_node_type(param.hir_id).to_string() == SOROBAN_ADDRESS { - self.linked_addresses - .push((AuthStatus { authed: false }, vec![param.pat.hir_id])); - } - }); - } - - fn get_node_type(&self, hir_id: HirId) -> Ty<'tcx> { - self.cx.typeck_results().node_type(hir_id) - } - - fn insert_new_address(&mut self, source_hir_id: HirId, target_hir_id: HirId) { - if let Some((_, linked_addresses)) = self - .linked_addresses - .iter_mut() - .find(|(_, addresses)| addresses.iter().any(|&id| id == source_hir_id)) - { - linked_addresses.push(target_hir_id); - } - } - - fn get_expr_hir_id(&self, expr: &Expr) -> Option { - let mut stack = vec![expr]; - - while let Some(current_expr) = stack.pop() { - match current_expr.kind { - ExprKind::MethodCall(_, call_expr, _, _) => stack.push(call_expr), - ExprKind::Path(QPath::Resolved(_, path)) => match path.res { - Res::Local(hir_id) => return Some(hir_id), - _ => continue, - }, - _ => continue, - } - } - - None - } - - fn get_linked_address(&self, id: HirId) -> Option<&(AuthStatus, Vec)> { - self.linked_addresses.iter().find(|(_, linked_addresses)| { - linked_addresses - .iter() - .any(|&address_hir_id| address_hir_id == id) - }) - } - - fn auth_address(&mut self, id: HirId) { - self.linked_addresses - .iter_mut() - .for_each(|(auth_status, linked_addresses)| { - if linked_addresses - .iter() - .any(|&address_hir_id| address_hir_id == id) - { - auth_status.authed = true; - } - }); - } -} diff --git a/scout-audit-internal/src/detector.rs b/scout-audit-internal/src/detector.rs index 32bf721c..b6914bed 100644 --- a/scout-audit-internal/src/detector.rs +++ b/scout-audit-internal/src/detector.rs @@ -35,7 +35,6 @@ pub enum Detector { OverflowCheck, SetContractStorage, SorobanVersion, - UnprotectedMappingOperation, UnprotectedUpdateCurrentContractWasm, UnsafeExpect, UnsafeUnwrap, @@ -48,17 +47,16 @@ impl Detector { match self { Detector::AvoidCoreMemForget => AVOID_CORE_MEM_FORGET_LINT_MESSAGE, Detector::AvoidPanicError => AVOID_PANIC_ERROR_LINT_MESSAGE, - Detector::AvoidUnsafeBlock => AVOID_UNSAFE_BLOCK_LINT_MESSAGE, Detector::DivideBeforeMultiply => DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE, Detector::DosUnboundedOperation => DOS_UNBOUNDED_OPERATION_LINT_MESSAGE, Detector::InsufficientlyRandomValues => INSUFFICIENTLY_RANDOM_VALUES_LINT_MESSAGE, Detector::OverflowCheck => OVERFLOW_CHECK_LINT_MESSAGE, Detector::SetContractStorage => SET_CONTRACT_STORAGE_LINT_MESSAGE, Detector::SorobanVersion => SOROBAN_VERSION_LINT_MESSAGE, - Detector::UnprotectedMappingOperation => UNPROTECTED_MAPPING_OPERATION_LINT_MESSAGE, Detector::UnprotectedUpdateCurrentContractWasm => { UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE } + Detector::AvoidUnsafeBlock => AVOID_UNSAFE_BLOCK_LINT_MESSAGE, Detector::UnsafeExpect => UNSAFE_EXPECT_LINT_MESSAGE, Detector::UnsafeUnwrap => UNSAFE_UNWRAP_LINT_MESSAGE, Detector::UnusedReturnEnum => UNUSED_RETURN_ENUM_LINT_MESSAGE, diff --git a/scout-audit-internal/src/detector/lint_message.rs b/scout-audit-internal/src/detector/lint_message.rs index 01358a13..4cf87b5c 100644 --- a/scout-audit-internal/src/detector/lint_message.rs +++ b/scout-audit-internal/src/detector/lint_message.rs @@ -12,8 +12,6 @@ pub const DOS_UNBOUNDED_OPERATION_LINT_MESSAGE: &str = pub const OVERFLOW_CHECK_LINT_MESSAGE: &str = "Use `overflow-checks = true` in Cargo.toml profile"; pub const SET_CONTRACT_STORAGE_LINT_MESSAGE:&str = "Abitrary users should not have control over keys because it implies writing any value of left mapping, lazy variable, or the main struct of the contract located in position 0 of the storage"; pub const SOROBAN_VERSION_LINT_MESSAGE: &str = "Use the latest version of Soroban"; -pub const UNPROTECTED_MAPPING_OPERATION_LINT_MESSAGE: &str = - "This mapping operation is called without access control on a different key than the caller's address"; pub const UNPROTECTED_UPDATE_CURRENT_CONTRACT_LINT_MESSAGE: &str = "This update_current_contract_wasm is called without access control"; pub const UNSAFE_EXPECT_LINT_MESSAGE: &str = "Unsafe usage of `expect`"; diff --git a/test-cases/README.md b/test-cases/README.md index 9b9c6127..18afcac7 100644 --- a/test-cases/README.md +++ b/test-cases/README.md @@ -192,11 +192,6 @@ Using a pinned version of Soroban can be dangerous, as it may have bugs or secur We classified this issue, a deviation from best practices which could have security implications, under the [Best practices](#vulnerability-categories) category and assigned it an Enhancement severity. -### Unprotected mapping operation - -Modifying mappings with an arbitrary key given by the user could lead to unintented modifications of critical data, modifying data belonging to other users, causing denial of service, unathorized access, and other potential issues. - -This vulnerability falls under the [Validations and error handling](#vulnerability-categories) category and has a Critical severity. ### Unused return enum `Rust` messages can return a `Result` `enum` with a custom error type. This is diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml deleted file mode 100644 index 5e367c2e..00000000 --- a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/Cargo.toml +++ /dev/null @@ -1,31 +0,0 @@ - -[package] -name = "unprotected-mapping-operation" -version = "0.1.0" -edition = "2021" - -[lib] -crate-type = ["cdylib"] - -[dependencies] -soroban-sdk = "=20.0.0" - -[dev_dependencies] -soroban-sdk = { version = "=20.0.0", features = ["testutils"] } - -[features] -testutils = ["soroban-sdk/testutils"] - -[profile.release] -opt-level = "z" -overflow-checks = true -debug = 0 -strip = "symbols" -debug-assertions = false -panic = "abort" -codegen-units = 1 -lto = true - -[profile.release-with-logs] -inherits = "release" -debug-assertions = true diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs deleted file mode 100644 index f3c98e50..00000000 --- a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/remediated-example/src/lib.rs +++ /dev/null @@ -1,71 +0,0 @@ -#![no_std] - -use soroban_sdk::{contract, contractimpl, contracttype, symbol_short, Address, Env, Map, Symbol}; - -#[contract] -pub struct UnprotectedMappingOperation; - -#[contracttype] -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct State { - balances: Map, -} -const STATE: Symbol = symbol_short!("STATE"); - -#[contractimpl] -impl UnprotectedMappingOperation { - pub fn set_balance(env: Env, address: Address, balance: i128) -> State { - // Authenticate user - address.require_auth(); - - // Get the current state. - let mut state = Self::get_state(env.clone()); - - // Set the new account to have total supply if it doesn't exist. - if !state.balances.contains_key(address.clone()) { - state.balances.set(address, balance); - // Save the state. - env.storage().persistent().set(&STATE, &state); - } - - state - } - - /// Return the current state. - pub fn get_state(env: Env) -> State { - env.storage().persistent().get(&STATE).unwrap_or(State { - balances: Map::new(&env), - }) - } -} - -#[cfg(test)] -const TOTAL_SUPPLY: i128 = 200; - -#[cfg(test)] -mod tests { - - use soroban_sdk::Env; - - use crate::{UnprotectedMappingOperation, UnprotectedMappingOperationClient, TOTAL_SUPPLY}; - - #[test] - fn balance_of_works() { - // Given - let env = Env::default(); - let contract_id = env.register_contract(None, UnprotectedMappingOperation); - let client = UnprotectedMappingOperationClient::new(&env, &contract_id); - - // When - let state = client - .mock_all_auths() - .set_balance(&contract_id, &TOTAL_SUPPLY); - - // Then - let balance = state - .balances - .get(contract_id) - .expect("Contract should have a balance"); - assert_eq!(TOTAL_SUPPLY, balance); - } -} diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml deleted file mode 100644 index 5e367c2e..00000000 --- a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/Cargo.toml +++ /dev/null @@ -1,31 +0,0 @@ - -[package] -name = "unprotected-mapping-operation" -version = "0.1.0" -edition = "2021" - -[lib] -crate-type = ["cdylib"] - -[dependencies] -soroban-sdk = "=20.0.0" - -[dev_dependencies] -soroban-sdk = { version = "=20.0.0", features = ["testutils"] } - -[features] -testutils = ["soroban-sdk/testutils"] - -[profile.release] -opt-level = "z" -overflow-checks = true -debug = 0 -strip = "symbols" -debug-assertions = false -panic = "abort" -codegen-units = 1 -lto = true - -[profile.release-with-logs] -inherits = "release" -debug-assertions = true diff --git a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs b/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs deleted file mode 100644 index 34a88680..00000000 --- a/test-cases/unprotected-mapping-operation/unprotected-mapping-operation-1/vulnerable-example/src/lib.rs +++ /dev/null @@ -1,66 +0,0 @@ -#![no_std] - -use soroban_sdk::{contract, contractimpl, contracttype, symbol_short, Address, Env, Map, Symbol}; - -#[contract] -pub struct UnprotectedMappingOperation; - -#[contracttype] -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct State { - balances: Map, -} -const STATE: Symbol = symbol_short!("STATE"); - -#[contractimpl] -impl UnprotectedMappingOperation { - pub fn set_balance(env: Env, address: Address, balance: i128) -> State { - // Get the current state. - let mut state = Self::get_state(env.clone()); - - // Set the new account to have total supply if it doesn't exist. - if !state.balances.contains_key(address.clone()) { - state.balances.set(address, balance); - // Save the state. - env.storage().persistent().set(&STATE, &state); - } - - state - } - - /// Return the current state. - pub fn get_state(env: Env) -> State { - env.storage().persistent().get(&STATE).unwrap_or(State { - balances: Map::new(&env), - }) - } -} - -#[cfg(test)] -const TOTAL_SUPPLY: i128 = 200; - -#[cfg(test)] -mod tests { - - use soroban_sdk::Env; - - use crate::{UnprotectedMappingOperation, UnprotectedMappingOperationClient, TOTAL_SUPPLY}; - - #[test] - fn balance_of_works() { - // Given - let env = Env::default(); - let contract_id = env.register_contract(None, UnprotectedMappingOperation); - let client = UnprotectedMappingOperationClient::new(&env, &contract_id); - - // When - let state = client.set_balance(&contract_id, &TOTAL_SUPPLY); - - // Then - let balance = state - .balances - .get(contract_id) - .expect("Contract should have a balance"); - assert_eq!(TOTAL_SUPPLY, balance); - } -}