Merge pull request #217 from CoinFabrik/167-add-new-detector-and-test…

…-cases-for-incorrect-exponentiation

167 add new detector and test cases for incorrect exponentiation

.github/workflows/deploy-docs.yml

@@ -6,6 +6,7 @@ on:
       - main
     paths:
       - "docs/**"
+  workflow_dispatch:
 
 jobs:
   deploy:

.github/workflows/general-rust.yml

@@ -8,6 +8,7 @@ on:
       - "scripts/**"
       - "!detectors/**/*.md"
       - "!test-cases/**/*.md"
+  workflow_dispatch:
 
 env:
   CARGO_TERM_COLOR: always

.github/workflows/test-deploy-docs.yml

@@ -6,6 +6,7 @@ on:
       - main
     paths:
       - "docs/**"
+  workflow_dispatch:
 
 jobs:
   test-deploy:

.github/workflows/test-detectors.yml

@@ -8,6 +8,7 @@ on:
       - "scripts/**"
       - "!detectors/**/*.md"
       - "!test-cases/**/*.md"
+  workflow_dispatch:
 
 env:
   CARGO_TERM_COLOR: always
@@ -57,8 +58,10 @@ jobs:
           key: ${{ runner.os }}-tests-${{ hashFiles('**/Cargo.lock') }}
           restore-keys: ${{ runner.os }}-tests-
 
-      - name: Install Rust nightly
-        run: rustup install nightly-2023-12-16
+      - name: Install Rust nightly and add rust-src
+        run: |
+          rustup install nightly-2023-12-16
+          rustup component add rust-src --toolchain nightly-2023-12-16
 
       - name: Install dylint, dylint-link and cargo-scout-audit
         run: cargo +nightly-2023-12-16 install cargo-dylint dylint-link cargo-scout-audit
@@ -121,10 +124,7 @@ jobs:
           restore-keys: ${{ runner.os }}-tests-
 
       - name: Run unit and integration tests
-        run: |
-          rustup toolchain install nightly-2023-12-16-x86_64-apple-darwin
-          rustup component add rust-src --toolchain nightly-2023-12-16-x86_64-apple-darwin
-          python scripts/run-tests.py --detector=${{ matrix.detector }}
+        run: python scripts/run-tests.py --detector=${{ matrix.detector }}
 
   comment-on-pr:
     name: Comment on PR
@@ -139,13 +139,11 @@ jobs:
         id: ubuntu_status
         working-directory: build-status-ubuntu-latest
         run: echo "status=$(cat status-ubuntu-latest.txt)" >> $GITHUB_OUTPUT
-        continue-on-error: true
 
       - name: Read macOS build status
         id: macos_status
-        working-directory: build-status-macos-latest
-        run: echo "status=$(cat status-macos-latest.txt)" >> $GITHUB_OUTPUT
-        continue-on-error: true
+        working-directory: build-status-macos-13
+        run: echo "status=$(cat status-macos-13.txt)" >> $GITHUB_OUTPUT
 
       - name: Find comment
         id: find_comment

README.md

@@ -68,6 +68,7 @@ Currently Scout includes the following detectors.
 | [unrestricted-transfer-from](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unrestricted-transfer-from)                           | Avoid passing an user-defined parameter as a `from` field in transfer-from. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unrestricted-transfer-from/unrestricted-transfer-from-1)                   | Critical  |
 | [unsafe-map-get](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unsafe-map-get)                           | Inappropriate usage of the `get` method for `Map` in soroban | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unsafe-map-get/unsafe-map-get-1)              | Medium |
 | [zero-or-test-address](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/zero-or-test-address)                           | Avoid zero or test address assignment to prevent contract control loss. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/zero-or-test-address/zero-or-test-address-1)                   | Validations and error handling  |
+| [incorrect-exponentation](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/incorrect-exponentiation)                           | Warns against incorrect usage of ´^´.  | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1)                   | Critical  |
 
 ## Output formats
 
@@ -141,3 +142,4 @@ Our team has an academic background in computer science and mathematics, with wo
 ## License
 
 Scout is licensed and distributed under a MIT license. [Contact us](https://www.coinfabrik.com/) if you're looking for an exception to the terms.
+

detectors/incorrect-exponentiation/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "incorrect-exponentiation"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+scout-audit-clippy-utils = { workspace = true }
+dylint_linting = { workspace = true }
+if_chain = { workspace = true }
+
+
+[package.metadata.rust-analyzer]
+rustc_private = true

detectors/incorrect-exponentiation/src/lib.rs

@@ -0,0 +1,81 @@
+#![feature(rustc_private)]
+#![warn(unused_extern_crates)]
+
+extern crate rustc_hir;
+extern crate rustc_span;
+
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::intravisit::Visitor;
+use rustc_hir::intravisit::{walk_expr, FnKind};
+use rustc_hir::{Body, FnDecl};
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_span::Span;
+use scout_audit_clippy_utils::diagnostics::span_lint_and_help;
+
+const LINT_MESSAGE: &str = "'^' It is not an exponential operator. It is a bitwise XOR.";
+const LINT_HELP: &str = "If you want to use XOR, use bitxor(). If you want to raise a number use .checked_pow() or .pow() ";
+
+dylint_linting::declare_late_lint! {
+    pub INCORRECT_EXPONENTIATION,
+    Warn,
+    LINT_MESSAGE,
+    {
+        name: "Incorrect Exponentiation",
+        long_message: LINT_MESSAGE,
+        severity: "Critical",
+        help: "https://coinfabrik.github.io/scout/docs/vulnerabilities/incorrect-exponentiation",
+        vulnerability_class: "Arithmetic",
+    }
+
+}
+
+impl<'tcx> LateLintPass<'tcx> for IncorrectExponentiation {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        _: FnKind<'tcx>,
+        _: &'tcx FnDecl<'_>,
+        body: &'tcx Body<'_>,
+        _: Span,
+        _: LocalDefId,
+    ) {
+        struct IncorrectExponentiationStorage {
+            span: Vec<Span>,
+            incorrect_exponentiation: bool,
+        }
+
+        impl<'tcx> Visitor<'tcx> for IncorrectExponentiationStorage {
+            fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+                if let ExprKind::Binary(op, _, _) = &expr.kind {
+                    if op.node == rustc_hir::BinOpKind::BitXor {
+                        self.incorrect_exponentiation = true;
+                        self.span.push(expr.span);
+                    }
+                }
+
+                walk_expr(self, expr);
+            }
+        }
+
+        let mut expon_storage = IncorrectExponentiationStorage {
+            span: Vec::new(),
+            incorrect_exponentiation: false,
+        };
+
+        walk_expr(&mut expon_storage, body.value);
+
+        if expon_storage.incorrect_exponentiation {
+            for span in expon_storage.span.iter() {
+                span_lint_and_help(
+                    cx,
+                    INCORRECT_EXPONENTIATION,
+                    *span,
+                    LINT_MESSAGE,
+                    None,
+                    LINT_HELP,
+                );
+            }
+        }
+    }
+}

docs/docs/detectors/21-incorrect-exponentiation.md

@@ -0,0 +1,36 @@
+# Incorrect Exponentiation
+
+### What it does
+
+Warns about `^` being a `bit XOR` operation instead of an exponentiation.  
+
+### Why is this bad?
+
+It can introduce unexpected behaviour in the smart contract.
+
+#### More info
+
+- https://doc.rust-lang.org/std/ops/trait.BitXor.html#tymethod.bitxor
+
+### Example
+
+```rust
+    pub fn init(e: Env){
+        e.storage()
+            .instance()
+            .set::<DataKey, u128>(&DataKey::Data, &((255_u128 ^ 2) - 1));
+    }
+```
+Use instead:
+
+```rust
+     pub fn init(e: Env) {
+        e.storage()
+            .instance()
+            .set::<DataKey, u128>(&DataKey::Data, &(255_u128.pow(2) - 1));
+    }
+```
+
+### Implementation
+
+The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/incorrect-exponentiation).

docs/docs/vulnerabilities/21-incorrect-exponentiation.md

@@ -0,0 +1,51 @@
+# Incorrect Exponentiation
+
+## Description
+
+- Vulnerability Category: `Arithmetic`
+- Vulnerability Severity: `Critical`
+- Detectors: [`incorrect-exponentiation`](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/incorrect-exponentiation)
+- Test Cases: [`incorrect-exponentiation-1`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1)
+
+
+The operator `^` is not an exponential operator, it is a bitwise XOR. Make sure to use `pow()` instead for exponentiation. In case of performing a XOR operation, use `.bitxor()` for clarity.
+
+## Exploit Scenario
+
+In the following example, the `^` operand is being used for exponentiation. But in Rust, `^` is the operand for an XOR operation. If misused, this could lead to unexpected behaviour in our contract.
+
+```rust
+   pub fn exp_data_3(e: Env) -> u128 {
+        let mut data = e.storage()
+        .instance()
+        .get::<DataKey, u128>(&DataKey::Data)
+        .expect("Data not found");
+
+        data ^= 3;
+        data
+    }
+```
+
+The vulnerable code example can be found [`here`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/vulnerable-example).
+
+## Remediation
+
+A possible solution is to use the method `pow()`. But, if a XOR operation is wanted, `.bitxor()` method is recommended.
+
+```rust
+    pub fn exp_data_3(e: Env) -> u128 {
+        let data = e.storage()
+        .instance()
+        .get::<DataKey, u128>(&DataKey::Data)
+        .expect("Data not found");
+
+        data.pow(3)
+    }
+```
+
+The remediated code example can be found [`here`](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example).
+
+## References
+
+- https://doc.rust-lang.org/std/ops/trait.BitXor.html
+

docs/docs/vulnerabilities/README.md

@@ -257,3 +257,9 @@ Assigning a test address can also have similar implications, including the loss
 
 This vulnerability falls under the [Validations and error handling](#vulnerability-categories) category
 and has a Medium severity.
+
+### Incorrect Exponentiation
+
+It's common to use `^` for exponentiation. However in Rust, `^` is the XOR operator. If the `^` operator is used, it could lead to unexpected behavior in the contract. It's recommended to use the method `pow()` for exponentiation or `.bitxor()` for XOR operations.
+
+This vulnerability falls under the [Arithmetic](#vulnerability-categories) category and has a Critical severity.

scripts/run-fmt.py

@@ -21,10 +21,6 @@ def run_fmt(directories):
         for root, _, _ in os.walk(directory):
             if is_rust_project(root):
                 start_time = time.time()
-                returncode, _, stderr = run_subprocess(
-                    ["cargo", "fmt"],
-                    cwd=root,
-                )
                 returncode, _, stderr = run_subprocess(
                     ["cargo", "fmt", "--check"],
                     cwd=root,

scripts/utils.py

@@ -78,7 +78,7 @@ def print_results(returncode, error_message, check_type, root, elapsed_time):
     )
     if returncode != 0:
         print(f"\n{RED}{check_type.capitalize()} {issue_type} found in: {root}{ENDC}\n")
-        if not error_message is None:
+        if error_message is not None:
             for line in error_message.strip().split("\n"):
                 print(f"| {line}")
             print("\n")

test-cases/incorrect-exponentiation/Cargo.toml

@@ -0,0 +1,21 @@
+[workspace]
+exclude = [".cargo", "target"]
+members = ["incorrect-exponentiation-*/*"]
+resolver = "2"
+
+[workspace.dependencies]
+soroban-sdk = { version = "20.3.2" }
+
+[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/incorrect-exponentiation/incorrect-exponentiation-1/remediated-example/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "incorrect-exponentiation-remediated-1"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+crate-type = ["cdylib"]
+
+[dependencies]
+soroban-sdk = { workspace = true }
+
+[dev-dependencies]
+soroban-sdk = { workspace = true, features = ["testutils"] }
+
+[features]
+testutils = ["soroban-sdk/testutils"]