10 Unprotected update current contract wasm

.github/workflows/test-detectors.yml

@@ -99,6 +99,7 @@ jobs:
           [
             "divide-before-multiply",
             "overflow-check",
+            "unprotected-update-current-contract-wasm",
             "unsafe-expect",
             "unsafe-unwrap",
           ]

detectors/unprotected-update-current-contract-wasm/Cargo.toml

@@ -0,0 +1,21 @@
+[package]
+name = "unprotected-update-current-contract-wasm"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+clippy_utils = { workspace = true }
+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

detectors/unprotected-update-current-contract-wasm/src/lib.rs

@@ -0,0 +1,192 @@
+#![feature(rustc_private)]
+#![feature(let_chains)]
+
+extern crate rustc_ast;
+extern crate rustc_hir;
+extern crate rustc_middle;
+extern crate rustc_span;
+
+use rustc_hir::{
+    intravisit::{walk_expr, Visitor},
+    Expr, ExprKind,
+};
+use rustc_lint::{LateContext, LateLintPass};
+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;
+
+dylint_linting::impl_late_lint! {
+    pub UNPROTECTED_UPDATE_CURRENT_CONTRACT_WASM,
+    Warn,
+    Detector::UnprotectedUpdateCurrentContractWasm.get_lint_message(),
+    UnprotectedUpdateCurrentContractWasm::default()
+}
+
+#[derive(Default)]
+pub struct UnprotectedUpdateCurrentContractWasm {}
+impl UnprotectedUpdateCurrentContractWasm {
+    pub fn new() -> Self {
+        Self {}
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for UnprotectedUpdateCurrentContractWasm {
+    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,
+        localdef: rustc_span::def_id::LocalDefId,
+    ) {
+        struct UnprotectedUpdateFinder<'tcx, 'tcx_ref> {
+            cx: &'tcx_ref LateContext<'tcx>,
+            require_auth_def_id: Option<DefId>,
+            update_contract_def_id: Option<DefId>,
+        }
+
+        impl<'tcx> Visitor<'tcx> for UnprotectedUpdateFinder<'tcx, '_> {
+            fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+                if let ExprKind::MethodCall(path, receiver, ..) = expr.kind {
+                    if path.ident.name.to_string() == "require_auth" {
+                        self.require_auth_def_id =
+                            self.cx.typeck_results().type_dependent_def_id(expr.hir_id);
+                    } else if path.ident.name.to_string() == "update_current_contract_wasm"
+                        && let ExprKind::MethodCall(path2, ..) = receiver.kind
+                        && path2.ident.name.to_string() == "deployer"
+                    {
+                        self.update_contract_def_id =
+                            self.cx.typeck_results().type_dependent_def_id(expr.hir_id);
+                    }
+                }
+
+                walk_expr(self, expr);
+            }
+        }
+
+        let mut uuf_storage = UnprotectedUpdateFinder {
+            cx,
+            require_auth_def_id: None,
+            update_contract_def_id: None,
+        };
+
+        walk_expr(&mut uuf_storage, body.value);
+
+        let mir_body = cx.tcx.optimized_mir(localdef);
+
+        let spans = navigate_trough_basicblocks(
+            &mir_body.basic_blocks,
+            BasicBlock::from_u32(0),
+            false,
+            &uuf_storage,
+        );
+
+        for span in spans {
+            Detector::UnprotectedUpdateCurrentContractWasm.span_lint(
+                cx,
+                UNPROTECTED_UPDATE_CURRENT_CONTRACT_WASM,
+                span,
+            )
+        }
+
+        fn navigate_trough_basicblocks<'tcx>(
+            bbs: &'tcx BasicBlocks<'tcx>,
+            bb: BasicBlock,
+            auth_checked: bool,
+            uuf_storage: &UnprotectedUpdateFinder,
+        ) -> Vec<Span> {
+            let mut ret_vec: Vec<Span> = Vec::<Span>::new();
+            if bbs[bb].terminator.is_none() {
+                return ret_vec;
+            }
+            let mut checked = auth_checked;
+            match &bbs[bb].terminator().kind {
+                TerminatorKind::Call {
+                    func,
+                    target,
+                    fn_span,
+                    ..
+                } => {
+                    if let Operand::Constant(fn_const) = func
+                        && let Const::Val(_const, ty) = fn_const.const_
+                        && let TyKind::FnDef(def, _) = ty.kind()
+                    {
+                        if uuf_storage.require_auth_def_id.is_some_and(|f| f == *def) {
+                            checked = true;
+                        } else if uuf_storage
+                            .update_contract_def_id
+                            .is_some_and(|f| f == *def)
+                            && !checked
+                        {
+                            ret_vec.push(*fn_span);
+                        }
+                    }
+                    if let Some(utarget) = target {
+                        ret_vec.append(&mut navigate_trough_basicblocks(
+                            bbs,
+                            *utarget,
+                            checked,
+                            uuf_storage,
+                        ));
+                    }
+                }
+                TerminatorKind::SwitchInt { targets, .. } => {
+                    for target in targets.all_targets() {
+                        ret_vec.append(&mut navigate_trough_basicblocks(
+                            bbs,
+                            *target,
+                            checked,
+                            uuf_storage,
+                        ));
+                    }
+                }
+                TerminatorKind::Assert { target, .. }
+                | TerminatorKind::Goto { target, .. }
+                | TerminatorKind::Drop { target, .. } => {
+                    ret_vec.append(&mut navigate_trough_basicblocks(
+                        bbs,
+                        *target,
+                        checked,
+                        uuf_storage,
+                    ));
+                }
+                TerminatorKind::Yield { resume, .. } => {
+                    ret_vec.append(&mut navigate_trough_basicblocks(
+                        bbs,
+                        *resume,
+                        checked,
+                        uuf_storage,
+                    ));
+                }
+                TerminatorKind::FalseEdge { real_target, .. }
+                | TerminatorKind::FalseUnwind { real_target, .. } => {
+                    ret_vec.append(&mut navigate_trough_basicblocks(
+                        bbs,
+                        *real_target,
+                        checked,
+                        uuf_storage,
+                    ));
+                }
+                TerminatorKind::InlineAsm { destination, .. } => {
+                    if let Some(udestination) = destination {
+                        ret_vec.append(&mut navigate_trough_basicblocks(
+                            bbs,
+                            *udestination,
+                            checked,
+                            uuf_storage,
+                        ));
+                    }
+                }
+                TerminatorKind::Return
+                | TerminatorKind::Unreachable
+                | TerminatorKind::GeneratorDrop
+                | TerminatorKind::UnwindResume
+                | TerminatorKind::UnwindTerminate(_) => {}
+            }
+            ret_vec
+        }
+    }
+}

scout-audit-internal/src/detector.rs

@@ -28,6 +28,7 @@ use strum::{Display, EnumIter};
 pub enum Detector {
     DivideBeforeMultiply,
     OverflowCheck,
+    UnprotectedUpdateCurrentContractWasm,
     UnsafeExpect,
     UnsafeUnwrap,
 }
@@ -39,6 +40,9 @@ impl Detector {
             Detector::DivideBeforeMultiply => DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE,
             Detector::UnsafeExpect => UNSAFE_EXPECT_LINT_MESSAGE,
             Detector::OverflowCheck => OVERFLOW_CHECK_LINT_MESSAGE,
+            Detector::UnprotectedUpdateCurrentContractWasm => {
+                UNPROTECTED_UPDATE_CURRENT_CONTRACT_MESSAGE
+            }
             Detector::UnsafeUnwrap => UNSAFE_UNWRAP_MESSAGE,
         }
     }

scout-audit-internal/src/detector/lint_message.rs

@@ -1,5 +1,7 @@
 pub const DIVIDE_BEFORE_MULTIPLY_LINT_MESSAGE: &str =
     "Division before multiplication might result in a loss of precision";
 pub const OVERFLOW_CHECK_LINT_MESSAGE: &str = "Use `overflow-checks = true` in Cargo.toml profile";
+pub const UNPROTECTED_UPDATE_CURRENT_CONTRACT_MESSAGE: &str =
+    "This update_current_contract_wasm is called without access control";
 pub const UNSAFE_EXPECT_LINT_MESSAGE: &str = "Unsafe usage of `expect`";
 pub const UNSAFE_UNWRAP_MESSAGE: &str = "Unsafe usage of `unwrap`";

test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1/remediated-example/Cargo.toml

@@ -0,0 +1,31 @@
+
+[package]
+name = "unprotected-update-current-contract-wasm"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+soroban-sdk = "20.0.0-rc2"
+
+[dev_dependencies]
+soroban-sdk = { version = "20.0.0-rc2", 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

test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1/remediated-example/src/lib.rs

@@ -0,0 +1,30 @@
+#![no_std]
+
+use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env};
+
+#[contracttype]
+#[derive(Clone)]
+enum DataKey {
+    Admin,
+}
+
+#[contract]
+pub struct UpgradeableContract;
+
+#[contractimpl]
+impl UpgradeableContract {
+    pub fn init(e: Env, admin: Address) {
+        e.storage().instance().set(&DataKey::Admin, &admin);
+    }
+
+    pub fn version() -> u32 {
+        1
+    }
+
+    pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) {
+        let admin: Address = e.storage().instance().get(&DataKey::Admin).unwrap();
+        admin.require_auth();
+
+        e.deployer().update_current_contract_wasm(new_wasm_hash);
+    }
+}

test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1/vulnerable-example/Cargo.toml

@@ -0,0 +1,31 @@
+
+[package]
+name = "unprotected-update-current-contract-wasm"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+soroban-sdk = "20.0.0-rc2"
+
+[dev_dependencies]
+soroban-sdk = { version = "20.0.0-rc2", 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

test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1/vulnerable-example/src/lib.rs

@@ -0,0 +1,29 @@
+#![no_std]
+
+use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env};
+
+#[contracttype]
+#[derive(Clone)]
+enum DataKey {
+    Admin,
+}
+
+#[contract]
+pub struct UpgradeableContract;
+
+#[contractimpl]
+impl UpgradeableContract {
+    pub fn init(e: Env, admin: Address) {
+        e.storage().instance().set(&DataKey::Admin, &admin);
+    }
+
+    pub fn version() -> u32 {
+        1
+    }
+
+    pub fn upgrade(e: Env, new_wasm_hash: BytesN<32>) {
+        let admin: Address = e.storage().instance().get(&DataKey::Admin).unwrap();
+
+        e.deployer().update_current_contract_wasm(new_wasm_hash);
+    }
+}