diff --git a/.github/workflows/code-scanning-sanctifier.yml b/.github/workflows/code-scanning-sanctifier.yml new file mode 100644 index 00000000..568bd596 --- /dev/null +++ b/.github/workflows/code-scanning-sanctifier.yml @@ -0,0 +1,77 @@ +name: Code Scanning (Sanctifier + CodeQL) + +# Uploads Sanctifier's SARIF to GitHub Code Scanning and runs CodeQL alongside +# it. Both tools publish into the same Code Scanning view using distinct +# `category` values so their alerts never overwrite each other. + +on: + push: + branches: ["main"] + pull_request: + branches: ["main"] + schedule: + # Re-scan weekly so newly published rules/queries surface on existing code. + - cron: "0 6 * * 1" + +# Least-privilege defaults. `security-events: write` is required to upload SARIF. +permissions: + contents: read + security-events: write + +jobs: + # ── Sanctifier: Soroban-specific static analysis → SARIF ──────────────────── + sanctifier: + name: Sanctifier (Soroban) + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install stable Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.x" + + # Use the published composite action. It runs + # sanctifier analyze --format sarif --min-severity --exit-code + # writes SARIF to `sarif-output`, and uploads it to Code Scanning. + - name: Run Sanctifier + uses: HyperSafeD/Sanctifier@main + continue-on-error: true # keep the workflow green; review alerts in the UI + with: + path: . + format: sarif + min-severity: high + upload-sarif: "true" + sarif-output: sanctifier-results.sarif + + # ── CodeQL: GitHub's first-party analysis, runs alongside Sanctifier ──────── + codeql: + name: CodeQL + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + actions: read + strategy: + fail-fast: false + matrix: + # CodeQL supports JS/TS via the `frontend/` dashboard in this repo. + # Adjust the language list to match what you ship. + language: ["javascript-typescript"] + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Initialize CodeQL + uses: github/codeql-action/init@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + languages: ${{ matrix.language }} + + - name: Perform CodeQL analysis + uses: github/codeql-action/analyze@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/contracts-ci.yml b/.github/workflows/contracts-ci.yml index 314ea6c4..c7e585d4 100644 --- a/.github/workflows/contracts-ci.yml +++ b/.github/workflows/contracts-ci.yml @@ -52,6 +52,37 @@ jobs: - name: cargo check --workspace run: cargo check --workspace --locked + # ------------------------------------------------------------------------- + # SEP-41 compliance: one standardized conformance suite that every SEP-41 + # token contract must pass. Its dependency tree is `syn`-only (no Z3), so + # this job stays fast and self-contained. + # ------------------------------------------------------------------------- + sep41-compliance: + name: SEP-41 compliance suite + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install stable Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Cache cargo artifacts + uses: actions/cache@v5 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-sep41-compliance-${{ hashFiles('**/Cargo.lock') }} + restore-keys: ${{ runner.os }}-sep41-compliance- + + - name: Run SEP-41 compliance suite + run: cargo test -p sep41-compliance + # ------------------------------------------------------------------------- # Job 1: check + clippy + test for every contract in the workspace. # Uses fail-fast: false so a single failure surfaces all broken contracts. diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f55108c3..461c8c8b 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -244,3 +244,47 @@ jobs: - name: Run Z3 module boundary integration tests run: cargo test --test smt_module_boundaries_test -p sanctifier-core + + # --------------------------------------------------------------------------- + # Property-based tests — the analysis engine must not panic on any input. + # Generates random Rust source (structured Soroban contracts, free-form + # snippets, and arbitrary text) and runs every rule against it, asserting the + # result is always Ok(findings) or Err(parse_error). Runs as a SEPARATE job + # with a hard 60-second budget on the proptest run itself (10,000 cases). + # --------------------------------------------------------------------------- + proptest: + name: Property-Based Tests (proptest) + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install stable Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Install Z3 + run: | + sudo apt-get update + sudo apt-get install -y libz3-dev libdbus-1-dev libudev-dev pkg-config + + - name: Cache cargo registry & build artifacts + uses: actions/cache@v5 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-proptest-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo-proptest- + + # Compile separately so the 60-second budget below covers test execution + # only, not the build. + - name: Build property-test binary (release) + run: cargo test -p sanctifier-core --test proptest_analysis --release --no-run + + - name: Run property tests — 10,000 inputs, 60s budget + env: + PROPTEST_CASES: "10000" + run: timeout 60s cargo test -p sanctifier-core --test proptest_analysis --release diff --git a/Cargo.lock b/Cargo.lock index 9fadf338..83819a00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3204,6 +3204,7 @@ dependencies = [ "criterion", "insta", "proc-macro2", + "proptest", "quote", "regex", "serde", @@ -3234,7 +3235,9 @@ dependencies = [ name = "sanctifier-test-support" version = "0.1.0" dependencies = [ + "quote", "soroban-sdk", + "syn 2.0.117", ] [[package]] @@ -3317,6 +3320,13 @@ dependencies = [ "serde_core", ] +[[package]] +name = "sep41-compliance" +version = "0.1.0" +dependencies = [ + "sanctifier-test-support", +] + [[package]] name = "serde" version = "1.0.228" diff --git a/Cargo.toml b/Cargo.toml index 705b99db..3bd5a423 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ members = [ "contracts/vesting", "contracts/benchmark", "contracts/deposit-withdraw", + "contracts/sep41-compliance", ] resolver = "2" diff --git a/README.md b/README.md index 9b90ac4e..3dffae8f 100644 --- a/README.md +++ b/README.md @@ -37,18 +37,18 @@ Every finding has a stable code — `S001..S012` — so you can filter, suppress | Code | What it catches | Why it bites | |------|-----------------|--------------| -| `S001` | Missing `require_auth` on state-changing calls | Anyone can drain your contract | -| `S002` | `panic!` / `unwrap` / `expect` in contract paths | Locked state, no recovery | -| `S003` | Unchecked arithmetic — overflow, underflow, truncation | Silent loss-of-funds rounding | -| `S004` | Ledger entries pushing the size threshold | Refusal at write time, mid-tx | -| `S005` | Storage-key collisions between data paths | Cross-feature data corruption | -| `S006` | Unsafe patterns — including timestamp-as-randomness | Predictable winners, exploit replay | -| `S007` | Your custom YAML rules | Your house style, enforced | -| `S008` | Inconsistent or missing event emissions | Wallets and indexers go blind | -| `S009` | Unhandled `Result` return values | Silent failures masquerading as success | -| `S010` | Upgrade / admin / governance risk | Single-key takeover paths | -| `S011` | Z3-disproved invariants | Mathematical guarantees you don't have | -| `S012` | SEP-41 token interface deviations | Wallets reject your token | +| [`S001`](docs/rules/S001.md) | Missing `require_auth` on state-changing calls | Anyone can drain your contract | +| [`S002`](docs/rules/S002.md) | `panic!` / `unwrap` / `expect` in contract paths | Locked state, no recovery | +| [`S003`](docs/rules/S003.md) | Unchecked arithmetic — overflow, underflow, truncation | Silent loss-of-funds rounding | +| [`S004`](docs/rules/S004.md) | Ledger entries pushing the size threshold | Refusal at write time, mid-tx | +| [`S005`](docs/rules/S005.md) | Storage-key collisions between data paths | Cross-feature data corruption | +| [`S006`](docs/rules/S006.md) | Unsafe patterns — including timestamp-as-randomness | Predictable winners, exploit replay | +| [`S007`](docs/rules/S007.md) | Your custom YAML rules | Your house style, enforced | +| [`S008`](docs/rules/S008.md) | Inconsistent or missing event emissions | Wallets and indexers go blind | +| [`S009`](docs/rules/S009.md) | Unhandled `Result` return values | Silent failures masquerading as success | +| [`S010`](docs/rules/S010.md) | Upgrade / admin / governance risk | Single-key takeover paths | +| [`S011`](docs/rules/S011.md) | Z3-disproved invariants | Mathematical guarantees you don't have | +| [`S012`](docs/rules/S012.md) | SEP-41 token interface deviations | Wallets reject your token | Plus the community **vulnerability database** matches known CVE-style patterns (`SOL-2024-*`) against your AST — so a published exploit anywhere becomes a finding everywhere. diff --git a/contracts/sep41-compliance/Cargo.toml b/contracts/sep41-compliance/Cargo.toml new file mode 100644 index 00000000..1cd9db80 --- /dev/null +++ b/contracts/sep41-compliance/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "sep41-compliance" +version = "0.1.0" +edition = "2021" +publish = false +description = "Standardized SEP-41 token-interface compliance suite run against every Sanctifier token contract." + +# This crate carries no runtime library of its own — it exists purely to host +# the `cargo test -p sep41-compliance` conformance suite. The reusable harness +# lives in `sanctifier-test-support::sep41_compliance`. +[lib] +path = "src/lib.rs" + +[dependencies] +sanctifier-test-support = { workspace = true } + +[dev-dependencies] +sanctifier-test-support = { workspace = true } diff --git a/contracts/sep41-compliance/src/lib.rs b/contracts/sep41-compliance/src/lib.rs new file mode 100644 index 00000000..2e3a679d --- /dev/null +++ b/contracts/sep41-compliance/src/lib.rs @@ -0,0 +1,32 @@ +//! # `sep41-compliance` — the standard SEP-41 conformance suite +//! +//! Every SEP-41 token contract in this workspace must pass one shared, +//! standardized compliance suite rather than relying on per-contract, ad-hoc +//! tests. This crate is that suite's home. +//! +//! - The reusable, generic harness lives in +//! [`sanctifier_test_support::sep41_compliance`]. It takes a contract's +//! source and verifies all ten SEP-41 functions: presence, exact signature, +//! and caller authorization. +//! - The actual conformance tests live in `tests/compliance.rs` and run the +//! harness against several real and reference token contracts. +//! +//! Run the whole suite with: +//! +//! ```bash +//! cargo test -p sep41-compliance +//! ``` +//! +//! Adding a new token contract to the suite is one line in +//! `tests/compliance.rs`: +//! +//! ```rust,ignore +//! assert_compliant("my-new-token", include_str!("../../my-new-token/src/lib.rs")); +//! ``` +//! +//! Any deviation from the SEP-41 interface — a missing function, a wrong +//! parameter type, or a missing `require_auth` — fails the suite. + +/// Re-export of the conformance harness so downstream crates can depend on a +/// single, stable path (`sep41_compliance::assert_compliant`). +pub use sanctifier_test_support::sep41_compliance; diff --git a/contracts/sep41-compliance/tests/compliance.rs b/contracts/sep41-compliance/tests/compliance.rs new file mode 100644 index 00000000..25134f83 --- /dev/null +++ b/contracts/sep41-compliance/tests/compliance.rs @@ -0,0 +1,103 @@ +//! Standardized SEP-41 compliance suite. +//! +//! This is the single, shared conformance suite that every SEP-41 token in the +//! workspace must pass. It runs the generic harness in +//! `sanctifier_test_support::sep41_compliance` against each token's source. +//! +//! Run it with: +//! +//! ```bash +//! cargo test -p sep41-compliance +//! ``` +//! +//! ## Adding a contract +//! +//! Add one `assert_compliant(...)` line in [`compliant_contracts`] pointing at +//! the contract's source. The harness verifies all ten SEP-41 functions — +//! presence, exact signature, and caller authorization — and any deviation +//! fails the test. + +use sanctifier_test_support::sep41_compliance::{ + assert_compliant, assert_deviates, check, IssueKind, REQUIRED_FUNCTION_COUNT, +}; + +/// Every contract here is asserted to be a fully compliant SEP-41 token. +/// +/// The suite intentionally covers **more than three** token contracts: +/// +/// 1. `my-contract` — a real, in-repo SEP-41 token implementation. +/// 2. `amm-lp-token` — reference LP token an `amm-pool` mints for liquidity +/// shares (`contracts/amm-pool`). +/// 3. `deposit-receipt-token` — reference receipt token a `deposit-withdraw` +/// vault mints for depositor claims (`contracts/deposit-withdraw`); also +/// exercises the `require_auth_for_args` authorization variant. +fn compliant_contracts() -> Vec<(&'static str, &'static str)> { + vec![ + ("my-contract", include_str!("../../my-contract/src/lib.rs")), + ("amm-lp-token", include_str!("fixtures/amm_lp_token.rs")), + ( + "deposit-receipt-token", + include_str!("fixtures/deposit_receipt_token.rs"), + ), + ] +} + +#[test] +fn suite_covers_at_least_three_contracts() { + assert!( + compliant_contracts().len() >= 3, + "the SEP-41 compliance suite must run against at least 3 contracts" + ); +} + +#[test] +fn sep41_interface_has_ten_functions() { + assert_eq!(REQUIRED_FUNCTION_COUNT, 10); +} + +/// Every compliant contract passes the full SEP-41 conformance check. +#[test] +fn all_token_contracts_are_sep41_compliant() { + for (name, source) in compliant_contracts() { + assert_compliant(name, source); + } +} + +// ── Negative tests: any deviation must fail ───────────────────────────────── +// +// These prove the suite is not vacuously green — a contract that drifts from +// the SEP-41 interface is reliably caught. + +#[test] +fn missing_function_fails_compliance() { + assert_deviates( + "noncompliant-missing-function", + include_str!("fixtures/noncompliant_missing_function.rs"), + IssueKind::MissingFunction, + ); +} + +#[test] +fn wrong_signature_fails_compliance() { + assert_deviates( + "noncompliant-wrong-signature", + include_str!("fixtures/noncompliant_wrong_signature.rs"), + IssueKind::SignatureMismatch, + ); +} + +#[test] +fn missing_authorization_fails_compliance() { + assert_deviates( + "noncompliant-missing-auth", + include_str!("fixtures/noncompliant_missing_auth.rs"), + IssueKind::AuthorizationMismatch, + ); +} + +/// A non-token contract is never reported as a compliant SEP-41 token. +#[test] +fn non_token_contract_is_not_compliant() { + let report = check("pub struct C; impl C { pub fn ping(e: Env) -> u32 { 0 } }"); + assert!(!report.compliant); +} diff --git a/contracts/sep41-compliance/tests/fixtures/amm_lp_token.rs b/contracts/sep41-compliance/tests/fixtures/amm_lp_token.rs new file mode 100644 index 00000000..80be883d --- /dev/null +++ b/contracts/sep41-compliance/tests/fixtures/amm_lp_token.rs @@ -0,0 +1,130 @@ +//! Reference SEP-41 LP (liquidity-provider) token, as an `amm-pool` would mint +//! to represent a share of pooled reserves. +//! +//! This fixture is a **complete, compliant** SEP-41 token used by the +//! `sep41-compliance` suite as a known-good contract. It mirrors the share +//! token an AMM pool issues on `add_liquidity` and redeems on +//! `remove_liquidity`. +#![no_std] + +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, MuxedAddress, String}; + +#[contracttype] +enum DataKey { + Balance(Address), + Allowance(Address, Address), + Decimals, + Name, + Symbol, +} + +#[contract] +pub struct AmmLpToken; + +#[contractimpl] +impl AmmLpToken { + pub fn initialize(env: Env, decimals: u32, name: String, symbol: String) { + env.storage().instance().set(&DataKey::Decimals, &decimals); + env.storage().instance().set(&DataKey::Name, &name); + env.storage().instance().set(&DataKey::Symbol, &symbol); + } + + // ── SEP-41 ──────────────────────────────────────────────────────────────── + + pub fn allowance(env: Env, from: Address, spender: Address) -> i128 { + env.storage() + .persistent() + .get(&DataKey::Allowance(from, spender)) + .unwrap_or(0) + } + + pub fn approve( + env: Env, + from: Address, + spender: Address, + amount: i128, + expiration_ledger: u32, + ) { + from.require_auth(); + let _ = expiration_ledger; + env.storage() + .persistent() + .set(&DataKey::Allowance(from, spender), &amount); + } + + pub fn balance(env: Env, id: Address) -> i128 { + env.storage() + .persistent() + .get(&DataKey::Balance(id)) + .unwrap_or(0) + } + + pub fn transfer(env: Env, from: Address, to: MuxedAddress, amount: i128) { + from.require_auth(); + let to = to.address(); + let from_bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(from_bal - amount)); + let to_bal = Self::balance(env.clone(), to.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(to), &(to_bal + amount)); + } + + pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { + spender.require_auth(); + let allowed = Self::allowance(env.clone(), from.clone(), spender.clone()); + env.storage().persistent().set( + &DataKey::Allowance(from.clone(), spender), + &(allowed - amount), + ); + let from_bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(from_bal - amount)); + let to_bal = Self::balance(env.clone(), to.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(to), &(to_bal + amount)); + } + + pub fn burn(env: Env, from: Address, amount: i128) { + from.require_auth(); + let bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(bal - amount)); + } + + pub fn burn_from(env: Env, spender: Address, from: Address, amount: i128) { + spender.require_auth(); + let allowed = Self::allowance(env.clone(), from.clone(), spender.clone()); + env.storage().persistent().set( + &DataKey::Allowance(from.clone(), spender), + &(allowed - amount), + ); + let bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(bal - amount)); + } + + pub fn decimals(env: Env) -> u32 { + env.storage().instance().get(&DataKey::Decimals).unwrap_or(7) + } + + pub fn name(env: Env) -> String { + env.storage() + .instance() + .get(&DataKey::Name) + .unwrap_or_else(|| String::from_str(&env, "AMM LP")) + } + + pub fn symbol(env: Env) -> String { + env.storage() + .instance() + .get(&DataKey::Symbol) + .unwrap_or_else(|| String::from_str(&env, "LP")) + } +} diff --git a/contracts/sep41-compliance/tests/fixtures/deposit_receipt_token.rs b/contracts/sep41-compliance/tests/fixtures/deposit_receipt_token.rs new file mode 100644 index 00000000..45fc426d --- /dev/null +++ b/contracts/sep41-compliance/tests/fixtures/deposit_receipt_token.rs @@ -0,0 +1,115 @@ +//! Reference SEP-41 deposit-receipt token, as a `deposit-withdraw` vault would +//! mint to represent a depositor's claim on the underlying balance. +//! +//! Complete and compliant — used by the `sep41-compliance` suite as a +//! known-good contract. Uses `require_auth_for_args` on `transfer` to exercise +//! the harness's recognition of that authorization variant. +#![no_std] + +use soroban_sdk::{ + contract, contractimpl, contracttype, Address, Env, IntoVal, MuxedAddress, String, +}; + +#[contracttype] +enum DataKey { + Balance(Address), + Allowance(Address, Address), +} + +#[contract] +pub struct DepositReceiptToken; + +#[contractimpl] +impl DepositReceiptToken { + pub fn allowance(env: Env, from: Address, spender: Address) -> i128 { + env.storage() + .persistent() + .get(&DataKey::Allowance(from, spender)) + .unwrap_or(0) + } + + pub fn approve( + env: Env, + from: Address, + spender: Address, + amount: i128, + expiration_ledger: u32, + ) { + from.require_auth(); + let _ = expiration_ledger; + env.storage() + .persistent() + .set(&DataKey::Allowance(from, spender), &amount); + } + + pub fn balance(env: Env, id: Address) -> i128 { + env.storage() + .persistent() + .get(&DataKey::Balance(id)) + .unwrap_or(0) + } + + pub fn transfer(env: Env, from: Address, to: MuxedAddress, amount: i128) { + // Exercise the `require_auth_for_args` authorization variant. + from.require_auth_for_args((&from, &amount).into_val(&env)); + let to = to.address(); + let from_bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(from_bal - amount)); + let to_bal = Self::balance(env.clone(), to.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(to), &(to_bal + amount)); + } + + pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { + spender.require_auth(); + let allowed = Self::allowance(env.clone(), from.clone(), spender.clone()); + env.storage().persistent().set( + &DataKey::Allowance(from.clone(), spender), + &(allowed - amount), + ); + let from_bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(from_bal - amount)); + let to_bal = Self::balance(env.clone(), to.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(to), &(to_bal + amount)); + } + + pub fn burn(env: Env, from: Address, amount: i128) { + from.require_auth(); + let bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(bal - amount)); + } + + pub fn burn_from(env: Env, spender: Address, from: Address, amount: i128) { + spender.require_auth(); + let allowed = Self::allowance(env.clone(), from.clone(), spender.clone()); + env.storage().persistent().set( + &DataKey::Allowance(from.clone(), spender), + &(allowed - amount), + ); + let bal = Self::balance(env.clone(), from.clone()); + env.storage() + .persistent() + .set(&DataKey::Balance(from), &(bal - amount)); + } + + pub fn decimals(_env: Env) -> u32 { + 7 + } + + pub fn name(env: Env) -> String { + String::from_str(&env, "Deposit Receipt") + } + + pub fn symbol(env: Env) -> String { + String::from_str(&env, "dRCPT") + } +} diff --git a/contracts/sep41-compliance/tests/fixtures/noncompliant_missing_auth.rs b/contracts/sep41-compliance/tests/fixtures/noncompliant_missing_auth.rs new file mode 100644 index 00000000..5f8781ca --- /dev/null +++ b/contracts/sep41-compliance/tests/fixtures/noncompliant_missing_auth.rs @@ -0,0 +1,23 @@ +//! NEGATIVE fixture: `transfer_from` never authorizes `spender`, so any caller +//! could move another account's tokens. The suite must flag this as +//! `AuthorizationMismatch`. +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env, MuxedAddress, String}; + +#[contract] +pub struct MissingAuth; + +#[contractimpl] +impl MissingAuth { + pub fn allowance(_e: Env, _from: Address, _spender: Address) -> i128 { 0 } + pub fn approve(_e: Env, from: Address, _spender: Address, _amount: i128, _exp: u32) { from.require_auth(); } + pub fn balance(_e: Env, _id: Address) -> i128 { 0 } + pub fn transfer(_e: Env, from: Address, _to: MuxedAddress, _amount: i128) { from.require_auth(); } + // No `spender.require_auth()` — the authorization gap. + pub fn transfer_from(_e: Env, spender: Address, _from: Address, _to: Address, _amount: i128) { let _ = spender; } + pub fn burn(_e: Env, from: Address, _amount: i128) { from.require_auth(); } + pub fn burn_from(_e: Env, spender: Address, _from: Address, _amount: i128) { spender.require_auth(); } + pub fn decimals(_e: Env) -> u32 { 7 } + pub fn name(e: Env) -> String { String::from_str(&e, "X") } + pub fn symbol(e: Env) -> String { String::from_str(&e, "X") } +} diff --git a/contracts/sep41-compliance/tests/fixtures/noncompliant_missing_function.rs b/contracts/sep41-compliance/tests/fixtures/noncompliant_missing_function.rs new file mode 100644 index 00000000..38f35288 --- /dev/null +++ b/contracts/sep41-compliance/tests/fixtures/noncompliant_missing_function.rs @@ -0,0 +1,21 @@ +//! NEGATIVE fixture: a token that is missing the `burn_from` SEP-41 function. +//! The compliance suite must flag this as `MissingFunction`. +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env, MuxedAddress, String}; + +#[contract] +pub struct MissingFn; + +#[contractimpl] +impl MissingFn { + pub fn allowance(_e: Env, _from: Address, _spender: Address) -> i128 { 0 } + pub fn approve(_e: Env, from: Address, _spender: Address, _amount: i128, _exp: u32) { from.require_auth(); } + pub fn balance(_e: Env, _id: Address) -> i128 { 0 } + pub fn transfer(_e: Env, from: Address, _to: MuxedAddress, _amount: i128) { from.require_auth(); } + pub fn transfer_from(_e: Env, spender: Address, _from: Address, _to: Address, _amount: i128) { spender.require_auth(); } + pub fn burn(_e: Env, from: Address, _amount: i128) { from.require_auth(); } + // `burn_from` intentionally omitted. + pub fn decimals(_e: Env) -> u32 { 7 } + pub fn name(e: Env) -> String { String::from_str(&e, "X") } + pub fn symbol(e: Env) -> String { String::from_str(&e, "X") } +} diff --git a/contracts/sep41-compliance/tests/fixtures/noncompliant_wrong_signature.rs b/contracts/sep41-compliance/tests/fixtures/noncompliant_wrong_signature.rs new file mode 100644 index 00000000..3e06d01e --- /dev/null +++ b/contracts/sep41-compliance/tests/fixtures/noncompliant_wrong_signature.rs @@ -0,0 +1,22 @@ +//! NEGATIVE fixture: `transfer` takes `amount: i64` instead of `i128`, and +//! `balance` returns `u64` instead of `i128`. The suite must flag these as +//! `SignatureMismatch`. +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env, MuxedAddress, String}; + +#[contract] +pub struct WrongSig; + +#[contractimpl] +impl WrongSig { + pub fn allowance(_e: Env, _from: Address, _spender: Address) -> i128 { 0 } + pub fn approve(_e: Env, from: Address, _spender: Address, _amount: i128, _exp: u32) { from.require_auth(); } + pub fn balance(_e: Env, _id: Address) -> u64 { 0 } // wrong return type + pub fn transfer(_e: Env, from: Address, _to: MuxedAddress, _amount: i64) { from.require_auth(); } // wrong amount type + pub fn transfer_from(_e: Env, spender: Address, _from: Address, _to: Address, _amount: i128) { spender.require_auth(); } + pub fn burn(_e: Env, from: Address, _amount: i128) { from.require_auth(); } + pub fn burn_from(_e: Env, spender: Address, _from: Address, _amount: i128) { spender.require_auth(); } + pub fn decimals(_e: Env) -> u32 { 7 } + pub fn name(e: Env) -> String { String::from_str(&e, "X") } + pub fn symbol(e: Env) -> String { String::from_str(&e, "X") } +} diff --git a/contracts/test-support/Cargo.toml b/contracts/test-support/Cargo.toml index 3fc8253b..2db69695 100644 --- a/contracts/test-support/Cargo.toml +++ b/contracts/test-support/Cargo.toml @@ -12,3 +12,8 @@ crate-type = ["rlib"] [dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } +# Used by the `sep41_compliance` conformance harness to parse contract source +# and verify the SEP-41 token interface. Host-side only (this crate is a +# dev-dependency of contracts), so it never reaches a WASM build. +syn = { version = "2.0", features = ["full", "extra-traits", "visit"] } +quote = "1.0" diff --git a/contracts/test-support/src/lib.rs b/contracts/test-support/src/lib.rs index be793c09..83ae76dd 100644 --- a/contracts/test-support/src/lib.rs +++ b/contracts/test-support/src/lib.rs @@ -21,4 +21,5 @@ pub mod asserts; pub mod env; +pub mod sep41_compliance; pub mod token; diff --git a/contracts/test-support/src/sep41_compliance.rs b/contracts/test-support/src/sep41_compliance.rs new file mode 100644 index 00000000..24c5081d --- /dev/null +++ b/contracts/test-support/src/sep41_compliance.rs @@ -0,0 +1,613 @@ +//! Generic SEP-41 token-interface **compliance test harness**. +//! +//! # Why this module exists +//! +//! Historically every token contract in `contracts/` shipped its own ad-hoc +//! SEP-41 tests. Those tests drifted: one contract checked `transfer_from` +//! authorization, another forgot it, a third never verified `burn_from` at +//! all. There was no single definition of "this contract is a conformant +//! SEP-41 token". +//! +//! This harness fixes that. It encodes the SEP-41 interface **once** and +//! exposes a generic entry point — [`assert_compliant`] — that any token +//! contract can be run through. Point it at a contract's source and it +//! verifies that **all ten** SEP-41 functions are present, have the exact +//! specified signatures, and authorize the correct caller. Any deviation +//! is a hard test failure. +//! +//! # Relationship to `sanctifier-core` +//! +//! The signature and authorization rules mirror Sanctifier's own `S012` +//! analysis pass (`sanctifier_core::sep41`). They are intentionally +//! reimplemented here with **no dependency on `sanctifier-core`** so the +//! compliance suite stays a lightweight, self-contained conformance check +//! that does not drag the analyzer's heavy toolchain (Z3, etc.) into every +//! contract's test build. If you change the SEP-41 spec in one place, +//! change it in both. +//! +//! # The SEP-41 interface +//! +//! | Function | Signature | Authorizes | +//! |-----------------|---------------------------------------------------------------------------|------------| +//! | `allowance` | `(env, from: Address, spender: Address) -> i128` | — | +//! | `approve` | `(env, from: Address, spender: Address, amount: i128, expiration_ledger: u32)` | `from` | +//! | `balance` | `(env, id: Address) -> i128` | — | +//! | `transfer` | `(env, from: Address, to: MuxedAddress, amount: i128)` | `from` | +//! | `transfer_from` | `(env, spender: Address, from: Address, to: Address, amount: i128)` | `spender` | +//! | `burn` | `(env, from: Address, amount: i128)` | `from` | +//! | `burn_from` | `(env, spender: Address, from: Address, amount: i128)` | `spender` | +//! | `decimals` | `(env) -> u32` | — | +//! | `name` | `(env) -> String` | — | +//! | `symbol` | `(env) -> String` | — | +//! +//! # Example +//! +//! ```rust,ignore +//! use sanctifier_test_support::sep41_compliance::assert_compliant; +//! +//! #[test] +//! fn my_token_is_sep41_compliant() { +//! assert_compliant("my-token", include_str!("../src/lib.rs")); +//! } +//! ``` + +use std::collections::HashSet; + +use quote::quote; +use syn::visit::{self, Visit}; +use syn::{parse_str, File, FnArg, Item, Pat, ReturnType, Type}; + +/// One required SEP-41 function and the contract it places on an implementer. +struct ExpectedFn { + /// Function name, e.g. `"transfer"`. + name: &'static str, + /// Ordered `(param_name, canonical_type)` pairs, including the leading `env`. + args: &'static [(&'static str, &'static str)], + /// Canonical return type (`"()"` for no return). + return_type: &'static str, + /// Index (into `args`) of the parameter that MUST be authorized, if any. + auth_param_index: Option, +} + +/// The canonical SEP-41 interface — the single source of truth for the suite. +const SEP41_FUNCTIONS: [ExpectedFn; 10] = [ + ExpectedFn { + name: "allowance", + args: &[("env", "Env"), ("from", "Address"), ("spender", "Address")], + return_type: "i128", + auth_param_index: None, + }, + ExpectedFn { + name: "approve", + args: &[ + ("env", "Env"), + ("from", "Address"), + ("spender", "Address"), + ("amount", "i128"), + ("expiration_ledger", "u32"), + ], + return_type: "()", + auth_param_index: Some(1), + }, + ExpectedFn { + name: "balance", + args: &[("env", "Env"), ("id", "Address")], + return_type: "i128", + auth_param_index: None, + }, + ExpectedFn { + name: "transfer", + args: &[ + ("env", "Env"), + ("from", "Address"), + ("to", "MuxedAddress"), + ("amount", "i128"), + ], + return_type: "()", + auth_param_index: Some(1), + }, + ExpectedFn { + name: "transfer_from", + args: &[ + ("env", "Env"), + ("spender", "Address"), + ("from", "Address"), + ("to", "Address"), + ("amount", "i128"), + ], + return_type: "()", + auth_param_index: Some(1), + }, + ExpectedFn { + name: "burn", + args: &[("env", "Env"), ("from", "Address"), ("amount", "i128")], + return_type: "()", + auth_param_index: Some(1), + }, + ExpectedFn { + name: "burn_from", + args: &[ + ("env", "Env"), + ("spender", "Address"), + ("from", "Address"), + ("amount", "i128"), + ], + return_type: "()", + auth_param_index: Some(1), + }, + ExpectedFn { + name: "decimals", + args: &[("env", "Env")], + return_type: "u32", + auth_param_index: None, + }, + ExpectedFn { + name: "name", + args: &[("env", "Env")], + return_type: "String", + auth_param_index: None, + }, + ExpectedFn { + name: "symbol", + args: &[("env", "Env")], + return_type: "String", + auth_param_index: None, + }, +]; + +/// The number of functions the SEP-41 interface requires. +pub const REQUIRED_FUNCTION_COUNT: usize = SEP41_FUNCTIONS.len(); + +/// The category of a single compliance failure. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum IssueKind { + /// A required function is absent. + MissingFunction, + /// A function exists but its signature does not match the specification. + SignatureMismatch, + /// A mutating function exists but does not authorize the required caller. + AuthorizationMismatch, +} + +/// A single SEP-41 compliance failure. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ComplianceIssue { + /// Name of the offending function. + pub function: String, + /// What kind of deviation this is. + pub kind: IssueKind, + /// The signature the SEP-41 spec requires. + pub expected: String, + /// The signature actually found (absent for [`IssueKind::MissingFunction`]). + pub actual: Option, + /// Human-readable explanation. + pub message: String, +} + +/// The outcome of checking one contract against the SEP-41 interface. +#[derive(Debug, Clone, Default)] +pub struct ComplianceReport { + /// `true` only when every required function is present, correctly typed, + /// and correctly authorized. + pub compliant: bool, + /// Names of the functions that passed every check. + pub verified: Vec, + /// Every deviation found. + pub issues: Vec, +} + +impl ComplianceReport { + /// Render a multi-line human-readable summary, used in assertion panics. + pub fn render(&self, contract_name: &str) -> String { + let mut out = format!( + "SEP-41 compliance report for `{contract_name}`: {} ({} / {} functions verified)\n", + if self.compliant { "COMPLIANT" } else { "NON-COMPLIANT" }, + self.verified.len(), + REQUIRED_FUNCTION_COUNT, + ); + for issue in &self.issues { + out.push_str(&format!( + " ✗ [{:?}] {}\n expected: {}\n", + issue.kind, issue.message, issue.expected + )); + if let Some(actual) = &issue.actual { + out.push_str(&format!(" found: {actual}\n")); + } + } + out + } +} + +/// A public method parsed out of a contract's `impl` block. +struct ParsedMethod { + arg_types: Vec, + arg_names: Vec>, + return_type: String, + /// Indices (into the canonicalized arg list) that the body authorizes. + authorized_params: HashSet, +} + +impl ParsedMethod { + fn signature(&self, name: &str) -> String { + let args = self + .arg_names + .iter() + .zip(self.arg_types.iter()) + .map(|(n, ty)| match n { + Some(n) => format!("{n}: {ty}"), + None => ty.clone(), + }) + .collect::>() + .join(", "); + format!("{name}({args}) -> {}", self.return_type) + } +} + +/// Check a contract's **source code** against the full SEP-41 interface. +/// +/// Returns a [`ComplianceReport`]. Source that does not parse as Rust yields a +/// non-compliant report with a single synthetic issue rather than panicking, so +/// the harness degrades gracefully on malformed fixtures. +pub fn check(source: &str) -> ComplianceReport { + let file = match parse_str::(source) { + Ok(file) => file, + Err(err) => { + return ComplianceReport { + compliant: false, + verified: Vec::new(), + issues: vec![ComplianceIssue { + function: "".to_string(), + kind: IssueKind::MissingFunction, + expected: "valid Rust source".to_string(), + actual: None, + message: format!("source did not parse as Rust: {err}"), + }], + }; + } + }; + + let methods = collect_public_methods(&file); + + let mut issues = Vec::new(); + let mut verified = Vec::new(); + + for expected in &SEP41_FUNCTIONS { + let expected_sig = render_expected(expected); + match methods.iter().find(|(name, _)| name.as_str() == expected.name) { + None => issues.push(ComplianceIssue { + function: expected.name.to_string(), + kind: IssueKind::MissingFunction, + expected: expected_sig, + actual: None, + message: format!("missing SEP-41 function `{}`", expected.name), + }), + Some((_, actual)) => { + let expected_types: Vec = + expected.args.iter().map(|(_, ty)| ty.to_string()).collect(); + + if actual.arg_types != expected_types || actual.return_type != expected.return_type + { + issues.push(ComplianceIssue { + function: expected.name.to_string(), + kind: IssueKind::SignatureMismatch, + expected: expected_sig, + actual: Some(actual.signature(expected.name)), + message: format!( + "function `{}` does not match the exact SEP-41 signature", + expected.name + ), + }); + continue; + } + + if let Some(auth_index) = expected.auth_param_index { + if !actual.authorized_params.contains(&auth_index) { + let authorizer = expected + .args + .get(auth_index) + .map(|(n, _)| *n) + .unwrap_or("caller"); + issues.push(ComplianceIssue { + function: expected.name.to_string(), + kind: IssueKind::AuthorizationMismatch, + expected: expected_sig, + actual: Some(actual.signature(expected.name)), + message: format!( + "function `{}` must authorize `{}` (call `{}.require_auth()`)", + expected.name, authorizer, authorizer + ), + }); + continue; + } + } + + verified.push(expected.name.to_string()); + } + } + } + + verified.sort(); + ComplianceReport { + compliant: issues.is_empty(), + verified, + issues, + } +} + +/// Assert that a contract's source is a fully compliant SEP-41 token. +/// +/// This is the **generic entry point** of the suite: hand it any token +/// contract's source and it runs every SEP-41 check. On any deviation it +/// panics with a full report, failing the test. +/// +/// # Panics +/// +/// Panics (failing the surrounding `#[test]`) if the contract is missing a +/// SEP-41 function, has a wrong signature, or fails to authorize a mutating +/// caller. +pub fn assert_compliant(contract_name: &str, source: &str) { + let report = check(source); + assert!(report.compliant, "{}", report.render(contract_name)); +} + +/// Assert that a contract is **not** SEP-41 compliant, and that at least one of +/// its deviations is of `expected_kind`. +/// +/// Used by the suite's negative tests to prove that deviations are actually +/// caught — i.e. that the harness fails loudly when an implementer drifts from +/// the spec. +/// +/// # Panics +/// +/// Panics if the contract is compliant, or if it is non-compliant for reasons +/// that do not include `expected_kind`. +pub fn assert_deviates(contract_name: &str, source: &str, expected_kind: IssueKind) { + let report = check(source); + assert!( + !report.compliant, + "expected `{contract_name}` to deviate from SEP-41, but it was compliant" + ); + assert!( + report.issues.iter().any(|i| i.kind == expected_kind), + "expected `{contract_name}` to have a {:?} issue, got: {}", + expected_kind, + report.render(contract_name) + ); +} + +// ── syn plumbing (mirrors sanctifier_core::sep41) ─────────────────────────── + +fn collect_public_methods(file: &File) -> Vec<(String, ParsedMethod)> { + let mut methods: Vec<(String, ParsedMethod)> = Vec::new(); + + for item in &file.items { + if let Item::Impl(item_impl) = item { + for impl_item in &item_impl.items { + if let syn::ImplItem::Fn(func) = impl_item { + if !matches!(func.vis, syn::Visibility::Public(_)) { + continue; + } + let name = func.sig.ident.to_string(); + if methods.iter().any(|(n, _)| n == &name) { + continue; // keep the first definition, like the analyzer + } + + let arg_types: Vec = func + .sig + .inputs + .iter() + .filter_map(|input| match input { + FnArg::Typed(typed) => Some(canonical_type(&typed.ty)), + FnArg::Receiver(_) => None, + }) + .collect(); + + let arg_names: Vec> = func + .sig + .inputs + .iter() + .filter_map(|input| match input { + FnArg::Typed(typed) => Some(pattern_name(&typed.pat)), + FnArg::Receiver(_) => None, + }) + .collect(); + + let mut visitor = RequireAuthVisitor::default(); + visitor.visit_block(&func.block); + + let authorized_params = arg_names + .iter() + .enumerate() + .filter_map(|(index, name)| { + name.as_ref() + .filter(|name| visitor.authorized_names.contains(*name)) + .map(|_| index) + }) + .collect(); + + methods.push(( + name, + ParsedMethod { + arg_types, + arg_names, + return_type: canonical_return_type(&func.sig.output), + authorized_params, + }, + )); + } + } + } + } + + methods +} + +fn render_expected(expected: &ExpectedFn) -> String { + let args = expected + .args + .iter() + .map(|(name, ty)| format!("{name}: {ty}")) + .collect::>() + .join(", "); + format!("{}({}) -> {}", expected.name, args, expected.return_type) +} + +fn canonical_return_type(output: &ReturnType) -> String { + match output { + ReturnType::Default => "()".to_string(), + ReturnType::Type(_, ty) => canonical_type(ty), + } +} + +fn canonical_type(ty: &Type) -> String { + match ty { + Type::Group(group) => canonical_type(&group.elem), + Type::Paren(paren) => canonical_type(&paren.elem), + Type::Reference(reference) => format!("&{}", canonical_type(&reference.elem)), + Type::Path(path) => path + .path + .segments + .last() + .map(|segment| segment.ident.to_string()) + .unwrap_or_else(|| simplify_tokens("e!(#ty).to_string())), + Type::Tuple(tuple) if tuple.elems.is_empty() => "()".to_string(), + _ => simplify_tokens("e!(#ty).to_string()), + } +} + +fn pattern_name(pat: &Pat) -> Option { + match pat { + Pat::Ident(ident) => Some(ident.ident.to_string()), + Pat::Reference(reference) => pattern_name(&reference.pat), + Pat::Type(typed) => pattern_name(&typed.pat), + Pat::Paren(paren) => pattern_name(&paren.pat), + _ => None, + } +} + +fn simplify_tokens(input: &str) -> String { + input.split_whitespace().collect::>().join(" ") +} + +#[derive(Default)] +struct RequireAuthVisitor { + authorized_names: HashSet, +} + +impl<'ast> Visit<'ast> for RequireAuthVisitor { + fn visit_expr_method_call(&mut self, node: &'ast syn::ExprMethodCall) { + let method_name = node.method.to_string(); + if method_name == "require_auth" || method_name == "require_auth_for_args" { + if let Some(name) = expr_identifier(&node.receiver) { + self.authorized_names.insert(name); + } + for arg in &node.args { + if let Some(name) = expr_identifier(arg) { + self.authorized_names.insert(name); + } + } + } + visit::visit_expr_method_call(self, node); + } + + fn visit_expr_call(&mut self, node: &'ast syn::ExprCall) { + if let syn::Expr::Path(path) = &*node.func { + if let Some(segment) = path.path.segments.last() { + let ident = segment.ident.to_string(); + if ident == "require_auth" || ident == "require_auth_for_args" { + for arg in &node.args { + if let Some(name) = expr_identifier(arg) { + self.authorized_names.insert(name); + } + } + } + } + } + visit::visit_expr_call(self, node); + } +} + +fn expr_identifier(expr: &syn::Expr) -> Option { + match expr { + syn::Expr::Path(path) => path + .path + .segments + .last() + .map(|segment| segment.ident.to_string()), + syn::Expr::Reference(reference) => expr_identifier(&reference.expr), + syn::Expr::Paren(paren) => expr_identifier(&paren.expr), + syn::Expr::Group(group) => expr_identifier(&group.expr), + syn::Expr::Unary(unary) => expr_identifier(&unary.expr), + syn::Expr::MethodCall(call) => expr_identifier(&call.receiver), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const COMPLIANT: &str = r#" + #![no_std] + use soroban_sdk::{contract, contractimpl, Address, Env, MuxedAddress, String}; + #[contract] + pub struct T; + #[contractimpl] + impl T { + pub fn allowance(env: Env, from: Address, spender: Address) -> i128 { 0 } + pub fn approve(env: Env, from: Address, spender: Address, amount: i128, expiration_ledger: u32) { from.require_auth(); } + pub fn balance(env: Env, id: Address) -> i128 { 0 } + pub fn transfer(env: Env, from: Address, to: MuxedAddress, amount: i128) { from.require_auth(); } + pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { spender.require_auth(); } + pub fn burn(env: Env, from: Address, amount: i128) { from.require_auth(); } + pub fn burn_from(env: Env, spender: Address, from: Address, amount: i128) { spender.require_auth(); } + pub fn decimals(env: Env) -> u32 { 7 } + pub fn name(env: Env) -> String { String::from_str(&env, "T") } + pub fn symbol(env: Env) -> String { String::from_str(&env, "T") } + } + "#; + + #[test] + fn compliant_token_passes() { + let report = check(COMPLIANT); + assert!(report.compliant, "{}", report.render("inline")); + assert_eq!(report.verified.len(), REQUIRED_FUNCTION_COUNT); + } + + #[test] + fn missing_function_is_flagged() { + let src = COMPLIANT.replace("pub fn burn_from", "pub fn unrelated_burn_from"); + assert_deviates("missing", &src, IssueKind::MissingFunction); + } + + #[test] + fn wrong_signature_is_flagged() { + // `amount: i64` instead of `i128`. + let src = COMPLIANT.replace( + "pub fn transfer(env: Env, from: Address, to: MuxedAddress, amount: i128)", + "pub fn transfer(env: Env, from: Address, to: MuxedAddress, amount: i64)", + ); + assert_deviates("wrong-sig", &src, IssueKind::SignatureMismatch); + } + + #[test] + fn missing_auth_is_flagged() { + let src = COMPLIANT.replace( + "pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { spender.require_auth(); }", + "pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { let _ = spender; }", + ); + assert_deviates("no-auth", &src, IssueKind::AuthorizationMismatch); + } + + #[test] + fn non_token_contract_is_not_compliant() { + let src = r#" + pub struct Counter; + impl Counter { + pub fn increment(env: Env) -> u32 { 0 } + pub fn get(env: Env) -> u32 { 0 } + } + "#; + assert!(!check(src).compliant); + } +} diff --git a/docs/INTEGRATION-GUIDE.md b/docs/INTEGRATION-GUIDE.md new file mode 100644 index 00000000..fbc89350 --- /dev/null +++ b/docs/INTEGRATION-GUIDE.md @@ -0,0 +1,330 @@ +# Integration Guide: Sanctifier with Third-Party Audit Tools + +This guide shows how to run **Sanctifier** alongside other security tools +(CodeQL, Semgrep, Slither) and consolidate their results into a single report, +GitHub Code Scanning view, or issue tracker (Jira / Linear). + +Sanctifier emits **SARIF 2.1.0**, the OASIS-standard format that GitHub Code +Scanning and most aggregation tooling consume natively. Because every tool in +this guide can produce SARIF, the integrations all reduce to the same pattern: +*produce SARIF → optionally merge → upload / route*. + +## Contents + +- [How Sanctifier produces SARIF](#how-sanctifier-produces-sarif) +- [Merging multiple SARIF files](#merging-multiple-sarif-files) +- [GitHub Code Scanning: Sanctifier + CodeQL](#github-code-scanning-sanctifier--codeql) +- [Semgrep + Sanctifier](#semgrep--sanctifier) +- [Slither in a multi-tool SARIF pipeline](#slither-in-a-multi-tool-sarif-pipeline) +- [Exporting findings to Jira / Linear](#exporting-findings-to-jira--linear) +- [Sample workflow files](#sample-workflow-files) + +--- + +## How Sanctifier produces SARIF + +Sanctifier writes SARIF to **stdout**; redirect it to a file. The verified +command (see [`README.md`](../README.md) line 94 and +[`tooling/sanctifier-cli/src/commands/analyze.rs`](../tooling/sanctifier-cli/src/commands/analyze.rs)): + +```bash +sanctifier analyze . --format sarif --min-severity high --exit-code > sanctifier-results.sarif +``` + +Flags used throughout this guide (all real — see `analyze.rs`): + +| Flag | Meaning | +|------|---------| +| `--format text\|json\|ndjson\|sarif` | Output format. `sarif` emits SARIF 2.1.0. Default: `text`. | +| `--min-severity critical\|high\|medium\|low` | Threshold that `--exit-code` reacts to. Default: `high`. | +| `--exit-code` | Exit `1` when findings meet/exceed the threshold (so CI can fail). | +| `--profile strict\|lenient\|ci\|audit` | Preset that overrides `--exit-code`/`--min-severity`. | +| `--webhook-url ` | POST a scan-completed summary to a webhook (repeatable). | +| `--webhook-secret ` | HMAC-SHA256 signing secret for `--webhook-url`. | + +> Note: there is **no** `--output ` flag — Sanctifier writes to stdout, so +> use shell redirection (`> file.sarif`) as shown above and in the action. + +The generated SARIF document has `tool.driver.name = "sanctifier"`, +`informationUri = https://github.com/HyperSafeD/Sanctifier`, and one +`runs[].results[]` entry per finding with `ruleId`, `level` +(`error`/`warning`/`note`), `message.text`, and a `physicalLocation` whose +`artifactLocation.uri` is the source file. See +[`docs/SARIF_METADATA.md`](./SARIF_METADATA.md) and +[`tooling/sanctifier-cli/src/commands/sarif.rs`](../tooling/sanctifier-cli/src/commands/sarif.rs) +for the exact shape. + +### The GitHub Action + +The repo ships a composite action ([`action.yml`](../action.yml)) that wraps the +CLI. Its inputs (use these exact names): + +| Input | Default | Description | +|-------|---------|-------------| +| `path` | `.` | Contract path to analyze. | +| `version` | _(tag)_ | `sanctifier-cli` version to install. | +| `min-severity` | `high` | `critical\|high\|medium\|low\|info`. | +| `format` | `sarif` | `text\|json\|sarif`. | +| `upload-sarif` | `"true"` | Upload SARIF to Code Scanning when `format: sarif`. | +| `sarif-output` | `sanctifier-results.sarif` | Path for the generated SARIF file. | +| `use-docker` | `"false"` | Run via the `ghcr.io/hypersafed/sanctifier` image. | +| `debug` | `"false"` | Verbose action logging. | + +Output: `findings-count`. When `upload-sarif: "true"`, the action uploads the +SARIF itself using `github/codeql-action/upload-sarif` — so you do **not** need a +separate upload step. (It skips upload on PRs from forks, where +`security-events: write` is unavailable.) + +--- + +## Merging multiple SARIF files + +There are two supported strategies. Prefer **(B)** for GitHub Code Scanning. + +### A. Merge into one file with the SARIF multitool + +[`@microsoft/sarif-multitool`](https://www.npmjs.com/package/@microsoft/sarif-multitool) +(the npm wrapper around the .NET SARIF SDK) merges multiple SARIF logs into a +single file — useful when a downstream system accepts only one upload, or for +archiving a combined audit artifact. + +```bash +# One-off, no install: +npx @microsoft/sarif-multitool merge \ + sanctifier-results.sarif \ + semgrep.sarif \ + codeql.sarif \ + --recurse true \ + --output combined.sarif +``` + +Each input tool's run is preserved as a separate `runs[]` entry inside +`combined.sarif`, so provenance (which tool found what) is not lost. + +### B. Upload each SARIF separately with a distinct `category` + +GitHub Code Scanning **accepts multiple SARIF uploads per commit** and keeps +them separate as long as each upload uses a unique `category`. This is the +recommended approach: no merge step, and each tool's alerts are tracked, +de-duplicated, and resolved independently. + +```yaml +- uses: github/codeql-action/upload-sarif@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + sarif_file: sanctifier-results.sarif + category: sanctifier # <- unique per tool +- uses: github/codeql-action/upload-sarif@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + sarif_file: semgrep.sarif + category: semgrep # <- different category +``` + +> If two uploads share a category, the later one **overwrites** the earlier +> one's alerts. Always give each tool its own category. + +--- + +## GitHub Code Scanning: Sanctifier + CodeQL + +A complete, working workflow lives at +[`.github/workflows/code-scanning-sanctifier.yml`](../.github/workflows/code-scanning-sanctifier.yml). +It runs Sanctifier and CodeQL as two parallel jobs and publishes both into Code +Scanning. The Sanctifier job uses the composite action (which uploads SARIF via +`github/codeql-action/upload-sarif` internally); the CodeQL job uses +`init` + `analyze`, where `analyze` uploads CodeQL's own SARIF with its own +category. + +Key requirements: + +```yaml +permissions: + contents: read + security-events: write # required to upload SARIF +``` + +The Sanctifier portion, inline: + +```yaml +- name: Run Sanctifier + uses: HyperSafeD/Sanctifier@main + continue-on-error: true + with: + path: . + format: sarif + min-severity: high + upload-sarif: "true" + sarif-output: sanctifier-results.sarif +``` + +If you would rather run the CLI directly and upload the SARIF yourself (for +extra flags, or to merge before upload), see +[`docs/integration-examples/sanctifier-cli-upload.yml`](./integration-examples/sanctifier-cli-upload.yml), +which uses `github/codeql-action/upload-sarif` with `category: sanctifier`. + +### Running CodeQL alongside + +The CodeQL job in the sample uses the standard pinned actions and a distinct +category, so its alerts coexist with Sanctifier's: + +```yaml +- uses: github/codeql-action/init@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + languages: javascript-typescript # adjust to your codebase +- uses: github/codeql-action/analyze@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + category: "/language:javascript-typescript" +``` + +> CodeQL does not currently analyze Soroban Rust contract logic the way +> Sanctifier does — that is exactly why you run them together. Point CodeQL at +> the languages it supports in your repo (e.g. the TypeScript dashboard under +> `frontend/`) and let Sanctifier cover the Soroban contracts. + +--- + +## Semgrep + Sanctifier + +Semgrep produces SARIF with `semgrep scan --sarif --output semgrep.sarif`. +Run it next to Sanctifier and upload each result set under its own category. + +Full workflow: +[`docs/integration-examples/semgrep-sanctifier.yml`](./integration-examples/semgrep-sanctifier.yml). + +Semgrep job, inline: + +```yaml +semgrep: + runs-on: ubuntu-latest + container: + image: semgrep/semgrep + steps: + - uses: actions/checkout@v6 + - run: semgrep scan --config p/rust --sarif --output semgrep.sarif + env: + SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }} + - uses: github/codeql-action/upload-sarif@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + sarif_file: semgrep.sarif + category: semgrep +``` + +`p/rust` is a reasonable default ruleset for Soroban's Rust codebase; swap in +your own Semgrep config or registry rulesets as needed. `SEMGREP_APP_TOKEN` is +optional and only needed for Semgrep Cloud Platform rulesets. + +If you prefer **one** combined file over two uploads, merge first: + +```bash +npx @microsoft/sarif-multitool merge semgrep.sarif sanctifier-results.sarif \ + --recurse true --output combined.sarif +# then a single upload of combined.sarif +``` + +--- + +## Slither in a multi-tool SARIF pipeline + +[Slither](https://github.com/crytic/slither) is an **EVM / Solidity** static +analyzer. It does **not** analyze Soroban (Rust/WASM) contracts, so it does not +overlap with Sanctifier. It is relevant only in **multi-chain** repositories +that contain both Solidity (EVM) and Soroban (Stellar) contracts — there, +Slither covers the Solidity side and Sanctifier covers the Soroban side, and +both feed the same Code Scanning view. + +Slither emits SARIF with the `--sarif` flag: + +```bash +slither . --sarif slither.sarif +``` + +Then upload it like any other tool, under its own category: + +```yaml +- name: Slither (Solidity / EVM contracts) + run: slither ./evm-contracts --sarif slither.sarif || true +- uses: github/codeql-action/upload-sarif@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + sarif_file: slither.sarif + category: slither +``` + +So a full multi-chain pipeline is just the same pattern repeated, one category +per tool: `sanctifier` (Soroban), `slither` (EVM), `semgrep`, `codeql`. + +--- + +## Exporting findings to Jira / Linear + +When you want tickets instead of (or in addition to) Code Scanning alerts, +parse the SARIF with `jq` and POST to the tracker's API. Sanctifier can also +fire a lightweight **scan-completed webhook** directly via +`--webhook-url`/`--webhook-secret` (HMAC-SHA256 signed), but that payload is a +*summary* (counts), not per-finding detail — for per-finding tickets, parse the +SARIF as below. + +A ready-to-adapt workflow is at +[`docs/integration-examples/sarif-to-jira-linear.yml`](./integration-examples/sarif-to-jira-linear.yml). +Store credentials as repository secrets; never inline them. + +### Extracting findings with jq + +```bash +jq -c '.runs[].results[] + | {ruleId, + level, + message: .message.text, + uri: .locations[0].physicalLocation.artifactLocation.uri}' \ + sanctifier-results.sarif +``` + +### Jira (REST API v3 — create issue) + +```bash +curl -sS -X POST "$JIRA_BASE_URL/rest/api/3/issue" \ + -u "$JIRA_EMAIL:$JIRA_API_TOKEN" \ + -H "Content-Type: application/json" \ + -d "$(jq -n \ + --arg proj "SEC" \ + --arg summary "[Sanctifier] $rule in $uri" \ + --arg body "$msg (file: $uri)" \ + '{fields: {project: {key: $proj}, + issuetype: {name: "Bug"}, + summary: $summary, + description: {type: "doc", version: 1, + content: [{type: "paragraph", + content: [{type: "text", text: $body}]}]}}}')" +``` + +### Linear (GraphQL — `issueCreate`) + +```bash +curl -sS -X POST https://api.linear.app/graphql \ + -H "Authorization: $LINEAR_API_KEY" \ + -H "Content-Type: application/json" \ + -d "$(jq -n \ + --arg team "$LINEAR_TEAM_ID" \ + --arg title "[Sanctifier] $rule in $uri" \ + --arg desc "$msg (file: $uri)" \ + '{query: "mutation($t:String!,$d:String!,$team:String!){issueCreate(input:{title:$t,description:$d,teamId:$team}){success}}", + variables: {t: $title, d: $desc, team: $team}}')" +``` + +> Tip: de-duplicate before creating tickets (e.g. search the tracker for an +> existing issue keyed on `ruleId + uri`) so re-runs don't open duplicates. The +> sample workflow leaves this to you to fit your team's conventions. + +--- + +## Sample workflow files + +| File | Integration | +|------|-------------| +| [`.github/workflows/code-scanning-sanctifier.yml`](../.github/workflows/code-scanning-sanctifier.yml) | **Tested, working** Code Scanning workflow: Sanctifier SARIF upload + CodeQL alongside. | +| [`docs/integration-examples/sanctifier-cli-upload.yml`](./integration-examples/sanctifier-cli-upload.yml) | Run the CLI directly and upload SARIF with `upload-sarif` + a category. | +| [`docs/integration-examples/semgrep-sanctifier.yml`](./integration-examples/semgrep-sanctifier.yml) | Semgrep + Sanctifier, each under its own category. | +| [`docs/integration-examples/sarif-to-jira-linear.yml`](./integration-examples/sarif-to-jira-linear.yml) | Parse SARIF with `jq` and open Jira / Linear tickets. | + +All four YAML files are valid (parse cleanly with `yaml.safe_load`) and use the +same action versions as the rest of the repo (`actions/checkout@v6`, +`dtolnay/rust-toolchain@stable`, `actions/setup-python@v5`, and the +SHA-pinned `github/codeql-action/*@…v3`). diff --git a/docs/integration-examples/sanctifier-cli-upload.yml b/docs/integration-examples/sanctifier-cli-upload.yml new file mode 100644 index 00000000..c4cebf5c --- /dev/null +++ b/docs/integration-examples/sanctifier-cli-upload.yml @@ -0,0 +1,50 @@ +name: Sanctifier (CLI) → Code Scanning + +# Alternative to the composite action: run the CLI directly, then upload the +# SARIF yourself with github/codeql-action/upload-sarif. Use this when you need +# full control over the command (custom vuln DB, profile, extra flags) or want +# to merge Sanctifier's SARIF with other tools before uploading. + +on: + push: + branches: ["main"] + pull_request: + branches: ["main"] + +permissions: + contents: read + security-events: write + +jobs: + scan: + name: Sanctifier CLI scan + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install stable Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Install Sanctifier CLI + # --no-default-features skips Z3 (every rule except S011 still runs) and + # avoids needing libz3-dev on the runner. Drop it if you want S011. + run: cargo install sanctifier-cli --locked --no-default-features + + - name: Run Sanctifier (write SARIF to stdout → file) + # The CLI prints SARIF to stdout; redirect it to a file. + # `|| true` keeps the step green so the SARIF still uploads even when + # --exit-code returns 1 because findings were found. + run: | + sanctifier analyze . \ + --format sarif \ + --min-severity high \ + --exit-code > sanctifier-results.sarif || true + + - name: Upload SARIF to Code Scanning + uses: github/codeql-action/upload-sarif@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + sarif_file: sanctifier-results.sarif + # A distinct category keeps these alerts separate from other tools + # (CodeQL, Semgrep) uploaded to the same repository. + category: sanctifier diff --git a/docs/integration-examples/sarif-to-jira-linear.yml b/docs/integration-examples/sarif-to-jira-linear.yml new file mode 100644 index 00000000..7033e500 --- /dev/null +++ b/docs/integration-examples/sarif-to-jira-linear.yml @@ -0,0 +1,89 @@ +name: Sanctifier → Jira / Linear + +# Runs Sanctifier with the CLI, then parses the SARIF with jq and opens a +# ticket per finding in Jira and/or Linear. This is a documented sample — wire +# in your own project keys / team IDs and store credentials as repo secrets. + +on: + workflow_dispatch: {} + schedule: + - cron: "0 7 * * 1" + +permissions: + contents: read + +jobs: + scan-and-export: + name: Scan and export findings + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install stable Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Install Sanctifier CLI + run: cargo install sanctifier-cli --locked --no-default-features + + - name: Run Sanctifier → SARIF + run: | + sanctifier analyze . \ + --format sarif \ + --min-severity high \ + --exit-code > sanctifier-results.sarif || true + + - name: Create Jira issues from findings + env: + JIRA_BASE_URL: ${{ secrets.JIRA_BASE_URL }} # https://your-org.atlassian.net + JIRA_EMAIL: ${{ secrets.JIRA_EMAIL }} + JIRA_API_TOKEN: ${{ secrets.JIRA_API_TOKEN }} + JIRA_PROJECT_KEY: SEC + run: | + set -euo pipefail + # Emit one compact JSON object per result: {ruleId, level, message, uri} + jq -c '.runs[].results[] + | {ruleId, level, message: .message.text, + uri: .locations[0].physicalLocation.artifactLocation.uri}' \ + sanctifier-results.sarif | while read -r finding; do + rule=$(echo "$finding" | jq -r '.ruleId') + msg=$(echo "$finding" | jq -r '.message') + uri=$(echo "$finding" | jq -r '.uri') + curl -sS -X POST "$JIRA_BASE_URL/rest/api/3/issue" \ + -u "$JIRA_EMAIL:$JIRA_API_TOKEN" \ + -H "Content-Type: application/json" \ + -d "$(jq -n \ + --arg proj "$JIRA_PROJECT_KEY" \ + --arg summary "[Sanctifier] $rule in $uri" \ + --arg body "$msg (file: $uri)" \ + '{fields: {project: {key: $proj}, + issuetype: {name: "Bug"}, + summary: $summary, + description: {type: "doc", version: 1, + content: [{type: "paragraph", + content: [{type: "text", text: $body}]}]}}}')" + done + + - name: Create Linear issues from findings + env: + LINEAR_API_KEY: ${{ secrets.LINEAR_API_KEY }} + LINEAR_TEAM_ID: ${{ secrets.LINEAR_TEAM_ID }} + run: | + set -euo pipefail + jq -c '.runs[].results[] + | {ruleId, message: .message.text, + uri: .locations[0].physicalLocation.artifactLocation.uri}' \ + sanctifier-results.sarif | while read -r finding; do + rule=$(echo "$finding" | jq -r '.ruleId') + msg=$(echo "$finding" | jq -r '.message') + uri=$(echo "$finding" | jq -r '.uri') + curl -sS -X POST https://api.linear.app/graphql \ + -H "Authorization: $LINEAR_API_KEY" \ + -H "Content-Type: application/json" \ + -d "$(jq -n \ + --arg team "$LINEAR_TEAM_ID" \ + --arg title "[Sanctifier] $rule in $uri" \ + --arg desc "$msg (file: $uri)" \ + '{query: "mutation($t:String!,$d:String!,$team:String!){issueCreate(input:{title:$t,description:$d,teamId:$team}){success}}", + variables: {t: $title, d: $desc, team: $team}}')" + done diff --git a/docs/integration-examples/semgrep-sanctifier.yml b/docs/integration-examples/semgrep-sanctifier.yml new file mode 100644 index 00000000..5603eb59 --- /dev/null +++ b/docs/integration-examples/semgrep-sanctifier.yml @@ -0,0 +1,62 @@ +name: Semgrep + Sanctifier + +# Runs Semgrep and Sanctifier in parallel and uploads each tool's SARIF to +# Code Scanning under its own `category`. GitHub keeps the two result sets +# separate, so you get a unified inbox without one tool clobbering the other. + +on: + push: + branches: ["main"] + pull_request: + branches: ["main"] + +permissions: + contents: read + security-events: write + +jobs: + semgrep: + name: Semgrep + runs-on: ubuntu-latest + container: + image: semgrep/semgrep + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Run Semgrep → SARIF + # `p/rust` is a sensible default ruleset for a Soroban (Rust) codebase. + run: semgrep scan --config p/rust --sarif --output semgrep.sarif + env: + SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }} + + - name: Upload Semgrep SARIF + uses: github/codeql-action/upload-sarif@dd903d2e4f5405488e5ef1422510ee31c8b32357 # v3 + with: + sarif_file: semgrep.sarif + category: semgrep + + sanctifier: + name: Sanctifier + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Install stable Rust toolchain + uses: dtolnay/rust-toolchain@stable + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.x" + + - name: Run Sanctifier + uses: HyperSafeD/Sanctifier@main + continue-on-error: true + with: + path: . + format: sarif + min-severity: high + upload-sarif: "true" + sarif-output: sanctifier-results.sarif diff --git a/docs/rules/S001.md b/docs/rules/S001.md new file mode 100644 index 00000000..383f87ab --- /dev/null +++ b/docs/rules/S001.md @@ -0,0 +1,91 @@ +# S001 — Missing Authorization Guard + +- **Category:** authentication +- **Severity:** Critical +- **Rule name:** `auth_gap` + +## What it detects + +S001 flags any public function inside an `impl` block that performs a **privileged operation** without first calling `require_auth()` or `require_auth_for_args()`. Privileged operations are: + +- **Storage mutation** — `.set()`, `.update()`, `.remove()`, or `.extend_ttl()` on `storage().persistent()`, `temporary()`, or `instance()`. +- **External contract call** — `.invoke_contract()`, or a method call on a receiver whose type name ends in `Client` / `_client`. + +Reserved Soroban entry-points (`__constructor`, `__check_auth`) are excluded automatically. Private (`fn`) functions and functions outside `impl` blocks are not checked. + +## Why it matters + +A public function that writes state or calls external contracts without checking the caller's identity can be invoked by **anyone** — draining balances, replacing the admin, or triggering arbitrary cross-contract logic. This is one of the most common critical findings in Soroban audits: if `require_auth` is missing, anyone can drain your contract. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Admin, +} + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + // S001: anyone can replace the admin — no auth guard. + pub fn set_admin(env: Env, new_admin: Address) { + env.storage().instance().set(&DataKey::Admin, &new_admin); + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Admin, +} + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn set_admin(env: Env, new_admin: Address) { + // Guard added: the current admin must authorize the change. + let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); + admin.require_auth(); + env.storage().instance().set(&DataKey::Admin, &new_admin); + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H` +- **Base score:** 9.8 +- **Rating:** Critical + +An unauthenticated network attacker can fully compromise integrity and availability (and confidentiality of controlled assets) with a single low-complexity transaction. + +## How to fix + +1. Call `addr.require_auth()` (or `addr.require_auth_for_args(...)`) as the **first statement** in every state-mutating or external-call public function. +2. Authorize the correct principal — typically the current admin for privileged operations, or the `from` address for value transfers. +3. Consider whether read-only functions that expose sensitive data also need authentication. +4. Re-run `sanctifier` to confirm the finding clears. + +## Related rules + +Related rules: [S010](S010.md), [S012](S012.md) + +## References + +- [Soroban authorization model](https://soroban.stellar.org/docs/fundamentals-and-concepts/authorization) +- [SEP-41 token standard](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0041.md) +- [CWE-862: Missing Authorization](https://cwe.mitre.org/data/definitions/862.html) diff --git a/docs/rules/S002.md b/docs/rules/S002.md new file mode 100644 index 00000000..7709b44d --- /dev/null +++ b/docs/rules/S002.md @@ -0,0 +1,87 @@ +# S002 — Panic Usage + +- **Category:** panic_handling +- **Severity:** Medium +- **Rule name:** `panic_detection` + +## What it detects + +S002 flags `panic!`, `.unwrap()`, and `.expect()` calls inside contract functions (functions within `impl` blocks and standalone module-level functions). Code under `#[test]` attributes and `#[cfg(test)]` modules is skipped to avoid false positives. + +## Why it matters + +A panic in a Soroban contract aborts the host invocation. There is no recovery path: the transaction reverts, and any state the function intended to repair stays broken. `.unwrap()` / `.expect()` on attacker-influenced values let a caller deliberately trigger aborts, locking contract state and turning a recoverable error into a hard failure with no graceful handling. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Balance(Address), +} + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn balance(env: Env, id: Address) -> i128 { + // S002: panics if the key is absent — abort with no recovery. + env.storage().persistent().get(&DataKey::Balance(id)).unwrap() + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Balance(Address), +} + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn balance(env: Env, id: Address) -> i128 { + // Default to zero instead of panicking on a missing entry. + env.storage() + .persistent() + .get(&DataKey::Balance(id)) + .unwrap_or(0) + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L` +- **Base score:** 5.3 +- **Rating:** Medium + +A network caller can trigger an abort with low complexity, but the impact is limited to availability of the affected operation (denial of service), with no direct confidentiality or integrity loss. + +## How to fix + +1. Replace `.unwrap()` / `.expect()` with `.unwrap_or()`, `.unwrap_or_default()`, or explicit `match` / `if let` handling. +2. Model recoverable conditions as a `Result` and propagate with `?`. +3. For cross-contract calls, prefer `try_invoke_contract` so callee failures return a `Result` instead of aborting. +4. Reserve panics for genuinely unreachable invariants, and document why. + +## Related rules + +Related rules: [S009](S009.md), [S003](S003.md) + +## References + +- [Soroban errors and panics](https://soroban.stellar.org/docs/learn/errors) +- [Rust `Result` and error handling](https://doc.rust-lang.org/book/ch09-00-error-handling.html) +- [CWE-248: Uncaught Exception](https://cwe.mitre.org/data/definitions/248.html) diff --git a/docs/rules/S003.md b/docs/rules/S003.md new file mode 100644 index 00000000..a6939865 --- /dev/null +++ b/docs/rules/S003.md @@ -0,0 +1,108 @@ +# S003 — Unchecked Arithmetic + +- **Category:** arithmetic +- **Severity:** Medium +- **Rule name:** `arithmetic_overflow` + +## What it detects + +S003 flags unchecked arithmetic operations that can overflow, underflow, or panic on a zero divisor: + +- Binary operators: `+`, `-`, `*`, `/`, `%`. +- Compound assignment: `+=`, `-=`, `*=`, `/=`, `%=`. +- Custom math methods and calls: `.mul_div()`, `.fixed_point_mul()`, `.fixed_point_div()`, `.div_ceil()` and their function-style equivalents. + +It does **not** flag `.checked_*` / `.saturating_*` methods, comparison and bitwise operators, string concatenation, array-index arithmetic (`buf[i + 1]`), or code under `#[test]` / `#[cfg(test)]`. Findings are deduplicated to at most one per `(function, operator)` pair. + +## Why it matters + +In Rust, integer overflow panics in debug builds and wraps silently in release builds. For a contract handling balances, either outcome is unacceptable: a panic locks state, and silent wraparound produces incorrect balances, unauthorized minting, or loss-of-funds rounding. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Balance(Address), +} + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn mint(env: Env, to: Address, amount: i128) { + let current: i128 = env + .storage() + .persistent() + .get(&DataKey::Balance(to.clone())) + .unwrap_or(0); + // S003: unchecked addition can overflow. + let new_balance = current + amount; + env.storage() + .persistent() + .set(&DataKey::Balance(to), &new_balance); + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Balance(Address), +} + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn mint(env: Env, to: Address, amount: i128) { + let current: i128 = env + .storage() + .persistent() + .get(&DataKey::Balance(to.clone())) + .unwrap_or(0); + // checked_add returns None on overflow. + let new_balance = current + .checked_add(amount) + .expect("mint: balance overflow"); + env.storage() + .persistent() + .set(&DataKey::Balance(to), &new_balance); + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:H/A:L` +- **Base score:** 5.9 +- **Rating:** Medium + +Exploitation usually requires crafted boundary inputs (higher attack complexity), but a successful overflow can corrupt balances (high integrity impact). + +## How to fix + +1. Replace unchecked operators with `.checked_add` / `.checked_sub` / `.checked_mul` / `.checked_div` / `.checked_rem` and handle the `None` case. +2. Where clamping is acceptable for non-financial values, use the `.saturating_*` variants. +3. For custom fixed-point math, use checked wrappers (e.g. `.checked_mul_div`). +4. Cast to a wider integer type before arithmetic when widening makes the operation provably safe. + +## Related rules + +Related rules: [S002](S002.md), [S011](S011.md) + +## References + +- [Rust integer overflow behavior](https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow) +- [CWE-190: Integer Overflow or Wraparound](https://cwe.mitre.org/data/definitions/190.html) +- [CWE-191: Integer Underflow](https://cwe.mitre.org/data/definitions/191.html) diff --git a/docs/rules/S004.md b/docs/rules/S004.md new file mode 100644 index 00000000..fd85cc4c --- /dev/null +++ b/docs/rules/S004.md @@ -0,0 +1,79 @@ +# S004 — Ledger Entry Size Risk + +- **Category:** storage_limits +- **Severity:** Medium +- **Rule name:** `ledger_size` + +## What it detects + +S004 estimates the serialized size of each `#[contracttype]` struct and enum and compares it against the configured ledger entry limit (default 64 kB, with an "approaching" threshold at 80%). It reports two levels: + +- **ExceedsLimit** — the estimated entry size is over the hard limit. +- **ApproachingLimit** — the estimated size is within the configured fraction of the limit. + +The limit and threshold are configurable via `.sanctify.toml`. + +## Why it matters + +Soroban rejects a ledger entry write when the serialized value exceeds the network's size limit. Because the rejection happens at write time, a struct that grows over time (for example a `Vec` of holders or a `Map` keyed by user) will eventually cause **mid-transaction failures** that brick the affected code path — often only on mainnet, under real load, after audit. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contracttype, Address, Map, String, Vec}; + +#[contracttype] +pub struct Registry { + // S004: an unbounded list/map embedded in one entry grows past the limit. + pub members: Vec
, + pub metadata: Map, + pub audit_log: Vec, +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contracttype, Address, String}; + +// Split the data across per-key entries instead of one giant struct. +#[contracttype] +pub enum DataKey { + Member(Address), // one small entry per member + Metadata(Address), // one small entry per member's metadata + MemberCount, // a single counter, not the full list +} + +#[contracttype] +pub struct MemberRecord { + pub joined_ledger: u32, + pub label: String, +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L` +- **Base score:** 5.3 +- **Rating:** Medium + +The dominant impact is availability: writes to an oversized entry fail, denying the affected operation. There is no direct confidentiality or integrity loss. + +## How to fix + +1. Reduce the size of the flagged `#[contracttype]` by removing or shrinking large fields. +2. Split monolithic structures across multiple keyed entries (one entry per user/item) instead of one collection in a single entry. +3. Store counters and indexes separately from bulk data. +4. If the estimate is a false positive for your network, raise `ledger_limit` in `.sanctify.toml`. + +## Related rules + +Related rules: [S005](S005.md), [S003](S003.md) + +## References + +- [Soroban storage and state archival](https://soroban.stellar.org/docs/learn/state-archival) +- [Soroban resource limits](https://developers.stellar.org/docs/networks/resource-limits-fees) +- [CWE-770: Allocation of Resources Without Limits](https://cwe.mitre.org/data/definitions/770.html) diff --git a/docs/rules/S005.md b/docs/rules/S005.md new file mode 100644 index 00000000..d15872c5 --- /dev/null +++ b/docs/rules/S005.md @@ -0,0 +1,88 @@ +# S005 — Storage Key Collision + +- **Category:** storage_keys +- **Severity:** Medium +- **Rule name:** `storage_collision` + +## What it detects + +S005 walks storage accesses and tracks the literal keys used per storage tier (`instance`, `persistent`, `temporary`). When the **same key value** is reused for **different data types** within the same tier, it reports a potential collision. It distinguishes keys by their `(storage_type, key_value)` pair and flags reuse across distinct value types or distinct data domains. + +## Why it matters + +A storage key uniquely identifies a ledger entry within a tier. If two unrelated pieces of data are written under the same key, one write silently overwrites the other, and a later read deserializes bytes that were never meant for it. The result is cross-feature data corruption that is hard to reproduce and easy to miss in review. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, symbol_short, Address, Env}; + +#[contract] +pub struct Vault; + +#[contractimpl] +impl Vault { + pub fn set_owner(env: Env, owner: Address) { + // S005: "data" reused for an Address ... + env.storage().instance().set(&symbol_short!("data"), &owner); + } + + pub fn set_total(env: Env, total: i128) { + // ... and here reused for an i128 — same key, different type. + env.storage().instance().set(&symbol_short!("data"), &total); + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, Env}; + +#[contracttype] +pub enum DataKey { + Owner, + Total, +} + +#[contract] +pub struct Vault; + +#[contractimpl] +impl Vault { + pub fn set_owner(env: Env, owner: Address) { + // Distinct, typed key variant per data domain. + env.storage().instance().set(&DataKey::Owner, &owner); + } + + pub fn set_total(env: Env, total: i128) { + env.storage().instance().set(&DataKey::Total, &total); + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:H/A:L` +- **Base score:** 5.9 +- **Rating:** Medium + +Triggering a meaningful collision generally requires a specific call ordering (higher complexity), but the consequence — silent overwrite of unrelated state — is a high integrity impact. + +## How to fix + +1. Use a single typed key enum (`#[contracttype] enum DataKey { ... }`) so each data domain has its own variant. +2. Give each domain a unique prefix; avoid reusing the same `symbol_short!` / `Symbol` for different value types. +3. Never key two different types off the same literal within one storage tier. + +## Related rules + +Related rules: [S004](S004.md), [S001](S001.md) + +## References + +- [Soroban storage tiers](https://soroban.stellar.org/docs/learn/persisting-data) +- [CWE-694: Use of Multiple Resources with Duplicate Identifier](https://cwe.mitre.org/data/definitions/694.html) +- [CWE-913: Improper Control of Dynamically-Managed Code Resources](https://cwe.mitre.org/data/definitions/913.html) diff --git a/docs/rules/S006.md b/docs/rules/S006.md new file mode 100644 index 00000000..cae8d9f8 --- /dev/null +++ b/docs/rules/S006.md @@ -0,0 +1,83 @@ +# S006 — Unsafe Pattern + +- **Category:** unsafe_patterns +- **Severity:** Medium +- **Rule name:** `unsafe_pattern` + +## What it detects + +S006 is the catch-all for potentially unsafe language or runtime patterns that don't have a more specific code. The flagship case is **timestamp-as-randomness**: using `env.ledger().timestamp()` as entropy. The detector fires when `env.ledger().timestamp()` appears inside: + +- a function whose name contains `rand`, `seed`, `pick`, or `winner`, or +- a variable binding whose name contains one of those tokens. + +Other unsafe runtime patterns surface under the same category, each with a suggested safe alternative. + +## Why it matters + +A block timestamp is not secret entropy. Validators can nudge `env.ledger().timestamp()` within a small window, so any "random" outcome derived from it — a lottery winner, a shuffled order, a seed — is predictable and manipulable. An attacker who can influence or predict the value can guarantee favorable outcomes and replay the exploit. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Env, Vec, Address}; + +#[contract] +pub struct Lottery; + +#[contractimpl] +impl Lottery { + pub fn pick_winner(env: Env, entrants: Vec
) -> Address { + // S006: timestamp used as the randomness source for the winner. + let seed = env.ledger().timestamp(); + let index = (seed % entrants.len() as u64) as u32; + entrants.get(index).unwrap() + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Env, Vec, Address}; + +#[contract] +pub struct Lottery; + +#[contractimpl] +impl Lottery { + pub fn pick_winner(env: Env, entrants: Vec
) -> Address { + // Use the host PRNG, which is seeded from unpredictable entropy. + let len = entrants.len(); + let index = env.prng().gen_range(0..len as u64) as u32; + entrants.get(index).unwrap() + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N` +- **Base score:** 7.5 +- **Rating:** High + +When the pattern is exploitable randomness, a low-complexity attacker can bias outcomes (high integrity impact), so the realized risk is high even though the catalog severity for the broad category is Medium. + +## How to fix + +1. Never use `env.ledger().timestamp()` (or sequence number) as a sole source of randomness. +2. Use the Soroban host PRNG (`env.prng()`), seeded from a fresh entropy source. +3. For high-value randomness, use a VRF oracle or commit-reveal scheme combining multiple unpredictable inputs. +4. For other flagged patterns, apply the suggested safe alternative or add an explicit safety check. + +## Related rules + +Related rules: [S003](S003.md), [S002](S002.md) + +## References + +- [Soroban PRNG](https://docs.rs/soroban-sdk/latest/soroban_sdk/struct.Prng.html) +- [CWE-330: Use of Insufficiently Random Values](https://cwe.mitre.org/data/definitions/330.html) +- [CWE-338: Use of Cryptographically Weak PRNG](https://cwe.mitre.org/data/definitions/338.html) diff --git a/docs/rules/S007.md b/docs/rules/S007.md new file mode 100644 index 00000000..7bf63f0d --- /dev/null +++ b/docs/rules/S007.md @@ -0,0 +1,78 @@ +# S007 — Custom Rule Match + +- **Category:** custom_rule +- **Severity:** Info +- **Rule name:** `custom_rule_match` + +## What it detects + +S007 is emitted when a **user-defined rule** matches the contract source. Teams author their own patterns (for example as YAML rules) describing strings, AST shapes, or banned APIs specific to their project. Whenever one of those custom patterns matches, Sanctifier reports it under S007 with the matched location, so house-style and policy checks ride the same engine and output format as the built-in rules. + +## Why it matters + +Every project has conventions that generic analyzers can't know — a deprecated internal helper, a forbidden direct storage access, a required naming prefix. S007 lets you encode those conventions once and enforce them in CI. The severity is informational because the meaning is defined entirely by your policy; what matters is that the match surfaces for review rather than slipping through. + +## Vulnerable example + +Suppose a project defines a custom rule that forbids direct calls to an internal `legacy_transfer` helper. The following source **matches** that rule and is flagged: + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env}; + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn pay(env: Env, from: Address, to: Address, amount: i128) { + from.require_auth(); + // S007: custom rule "no-legacy-transfer" matches this banned call. + legacy_transfer(&env, &from, &to, amount); + } +} +``` + +## Safe example + +The corrected version uses the approved API and no longer matches the custom rule: + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env}; + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn pay(env: Env, from: Address, to: Address, amount: i128) { + from.require_auth(); + // Approved transfer path — no custom-rule match. + transfer_v2(&env, &from, &to, amount); + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:N` +- **Base score:** 0.0 +- **Rating:** Low (Informational) + +A custom-rule match carries no inherent technical severity; its risk depends entirely on the policy it encodes. Teams that map a custom rule to a real vulnerability should rate that finding on its own merits. + +## How to fix + +1. Review the matched pattern and confirm whether it violates your project's security policy. +2. If it does, refactor the code to the approved alternative. +3. If the match is intentional and acceptable, document the exception or refine the custom rule to reduce false positives. + +## Related rules + +Related rules: [S006](S006.md), [S001](S001.md) + +## References + +- [Sanctifier rule authoring guide](../rule-authoring-guide.md) +- [CWE-710: Improper Adherence to Coding Standards](https://cwe.mitre.org/data/definitions/710.html) diff --git a/docs/rules/S008.md b/docs/rules/S008.md new file mode 100644 index 00000000..b017ccde --- /dev/null +++ b/docs/rules/S008.md @@ -0,0 +1,91 @@ +# S008 — Event Inconsistency + +- **Category:** events +- **Severity:** Low +- **Rule name:** `event_inconsistency` + +## What it detects + +S008 scans `env.events().publish(topics, data)` calls and checks: + +1. **Topic-count consistency** — when the same event name is published with a different number of topics in different places, it reports the mismatch (previous count vs. current count). +2. **Sub-optimal gas patterns** — string or `String` topics that could use `symbol_short!` for short identifiers are flagged as a gas-optimization suggestion. + +## Why it matters + +Wallets, indexers, and monitoring tools subscribe to events by topic shape. If the same logical event is sometimes published with two topics and sometimes with three, off-chain consumers can't reliably parse it — so wallets and indexers go blind to part of your contract's activity. Inconsistent or string-heavy topics also waste gas. The severity is Low because there is no on-chain fund risk, but the operational impact on integrations is real. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, symbol_short, Address, Env}; + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn mint(env: Env, to: Address, amount: i128) { + // Two topics for the "transfer" event here ... + env.events() + .publish((symbol_short!("transfer"), to), amount); + } + + pub fn transfer(env: Env, from: Address, to: Address, amount: i128) { + from.require_auth(); + // S008: three topics for the same "transfer" event name — inconsistent. + env.events() + .publish((symbol_short!("transfer"), from, to), amount); + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, symbol_short, Address, Env}; + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn mint(env: Env, to: Address, amount: i128) { + // Consistent (event, from, to) topic shape across all emitters. + let zero = Address::from_str(&env, "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF5"); + env.events() + .publish((symbol_short!("transfer"), zero, to), amount); + } + + pub fn transfer(env: Env, from: Address, to: Address, amount: i128) { + from.require_auth(); + env.events() + .publish((symbol_short!("transfer"), from, to), amount); + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N` +- **Base score:** 5.3 +- **Rating:** Low + +There is no on-chain integrity or availability impact; the consequence is degraded observability for off-chain consumers, mapped here as a minor confidentiality/visibility concern. The catalog severity is Low. + +## How to fix + +1. Standardize each event's topic layout and use the same number and order of topics everywhere it is emitted. +2. Define a single helper that publishes each event so the shape can't drift. +3. Prefer `symbol_short!` for short, fixed topic identifiers to save gas. + +## Related rules + +Related rules: [S010](S010.md), [S007](S007.md) + +## References + +- [Soroban events](https://soroban.stellar.org/docs/learn/events) +- [CWE-778: Insufficient Logging](https://cwe.mitre.org/data/definitions/778.html) diff --git a/docs/rules/S009.md b/docs/rules/S009.md new file mode 100644 index 00000000..f98e574b --- /dev/null +++ b/docs/rules/S009.md @@ -0,0 +1,94 @@ +# S009 — Unhandled Result + +- **Category:** logic +- **Severity:** Medium +- **Rule name:** `unhandled_result` + +## What it detects + +S009 flags expressions that produce a `Result` but never consume it — a `Result`-returning call used as a statement and discarded. The rule walks contract functions and reports calls whose `Result` value is dropped instead of being handled with `?`, `match`, `if let`, or an explicit `.unwrap()` / `.expect()`. + +## Why it matters + +A discarded `Result` means an error path is silently ignored: the call may have failed, but execution continues as if it succeeded. In a contract, that turns a failed sub-operation into a success the caller trusts — the classic "silent failure masquerading as success," which can leave balances, allowances, or flags in an inconsistent state with no signal that anything went wrong. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env}; + +fn record_payment(env: &Env, from: &Address, amount: i128) -> Result<(), Error> { + // ... may fail ... + Ok(()) +} + +pub enum Error { + Failed, +} + +#[contract] +pub struct Ledger; + +#[contractimpl] +impl Ledger { + pub fn pay(env: Env, from: Address, amount: i128) { + from.require_auth(); + // S009: the Result is discarded — a failure is silently ignored. + record_payment(&env, &from, amount); + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env}; + +fn record_payment(env: &Env, from: &Address, amount: i128) -> Result<(), Error> { + Ok(()) +} + +#[derive(Clone)] +pub enum Error { + Failed, +} + +#[contract] +pub struct Ledger; + +#[contractimpl] +impl Ledger { + pub fn pay(env: Env, from: Address, amount: i128) -> Result<(), Error> { + from.require_auth(); + // Propagate the error with `?` instead of dropping it. + record_payment(&env, &from, amount)?; + Ok(()) + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:H/A:N` +- **Base score:** 5.9 +- **Rating:** Medium + +Exploitation depends on forcing the ignored sub-operation to fail (higher complexity), but the consequence — committed state that assumes a failed step succeeded — is a high integrity impact. + +## How to fix + +1. Handle every `Result` with `?` to propagate, or `match` / `if let` to branch on the error. +2. If a failure is genuinely irrelevant, make the intent explicit (e.g. `let _ = ...;` with a comment) rather than silently dropping it. +3. For cross-contract calls, use `try_invoke_contract` and inspect the returned `Result`. + +## Related rules + +Related rules: [S002](S002.md), [S001](S001.md) + +## References + +- [Rust error handling](https://doc.rust-lang.org/book/ch09-00-error-handling.html) +- [CWE-252: Unchecked Return Value](https://cwe.mitre.org/data/definitions/252.html) +- [CWE-754: Improper Check for Unusual or Exceptional Conditions](https://cwe.mitre.org/data/definitions/754.html) diff --git a/docs/rules/S010.md b/docs/rules/S010.md new file mode 100644 index 00000000..a4c4eb6b --- /dev/null +++ b/docs/rules/S010.md @@ -0,0 +1,90 @@ +# S010 — Upgrade Risk + +- **Category:** upgrades +- **Severity:** Medium +- **Rule name:** `upgrade_risk` + +## What it detects + +S010 analyzes upgrade, admin, and initialization mechanisms. It walks `impl` blocks and reports: + +- **Governance** — an upgrade/admin function (e.g. `upgrade`, `set_admin`, anything matching the upgrade/admin heuristic) that mutates state **without `require_auth`**. +- **InitPattern** — an initialization function with **no re-init guard**, so it can be called more than once. +- **Timelock** — upgrade functions are checked for a delay/timelock reference so you can confirm the delay is actually enforced. + +## Why it matters + +Upgrade and admin paths are the keys to the kingdom. An unauthenticated `upgrade` lets anyone swap the contract's WASM; an `initialize` callable twice lets an attacker re-seize ownership; an instant, single-key upgrade with no timelock gives one compromised key total control. These are the single-key takeover paths that turn a small key leak into a full contract compromise. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env}; + +#[contracttype] +pub enum DataKey { + Admin, +} + +#[contract] +pub struct Upgradeable; + +#[contractimpl] +impl Upgradeable { + // S010: upgrade path mutates state with no require_auth and no timelock. + pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { + env.deployer().update_current_contract_wasm(new_wasm_hash); + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, contracttype, Address, BytesN, Env}; + +#[contracttype] +pub enum DataKey { + Admin, +} + +#[contract] +pub struct Upgradeable; + +#[contractimpl] +impl Upgradeable { + pub fn upgrade(env: Env, new_wasm_hash: BytesN<32>) { + // Require the admin (ideally a multisig/timelock account) to authorize. + let admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); + admin.require_auth(); + env.deployer().update_current_contract_wasm(new_wasm_hash); + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H` +- **Base score:** 9.8 +- **Rating:** Critical + +An unauthenticated upgrade path is a full takeover (Critical). The catalog assigns the category a Medium default because many S010 findings are governance-hardening recommendations (missing timelock, single-key admin) rather than open auth gaps; rate each finding by whether an auth guard is actually missing. + +## How to fix + +1. Add `require_auth` (or `require_auth_for_args`) on every upgrade and admin path, authorizing the admin principal. +2. Guard initialization with an early return when an init flag already exists in storage. +3. Use multi-signature governance and a timelock delay before upgrades take effect. +4. Emit an event on every upgrade/admin change so it can be monitored off-chain. + +## Related rules + +Related rules: [S001](S001.md), [S008](S008.md) + +## References + +- [Soroban contract upgrades](https://soroban.stellar.org/docs/build/guides/conventions/upgrading-contracts) +- [CWE-862: Missing Authorization](https://cwe.mitre.org/data/definitions/862.html) +- [CWE-269: Improper Privilege Management](https://cwe.mitre.org/data/definitions/269.html) diff --git a/docs/rules/S011.md b/docs/rules/S011.md new file mode 100644 index 00000000..ff9863f3 --- /dev/null +++ b/docs/rules/S011.md @@ -0,0 +1,80 @@ +# S011 — SMT Invariant Violation + +- **Category:** formal_verification +- **Severity:** High +- **Rule name:** `smt_invariant_violation` + +## What it detects + +S011 is produced by Sanctifier's formal-verification backend. It parses `#[invariant = "..."]` attributes on contract functions and feeds each invariant to the **Z3** SMT solver under a configurable timeout. A finding is emitted for every invariant that the solver **cannot prove safe** — either it found a concrete counterexample (the invariant can be violated) or it timed out. Invariants proved safe produce no finding. + +## Why it matters + +Unit tests check the cases you thought of; an SMT solver checks *all* of them within the modeled domain. When Z3 disproves an invariant — say, "total supply always equals the sum of balances" — it has produced a mathematical counterexample showing your contract can reach a state you believed impossible. These are exactly the deep accounting and conservation bugs that hand-review and fuzzing miss, so the severity is High and the counterexample trace is your direct path to the fix. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Env}; + +#[contract] +pub struct Bank; + +#[contractimpl] +impl Bank { + // S011: Z3 disproves this — withdraw can drive `balance` below zero, + // violating the stated invariant. + #[invariant = "balance >= 0"] + pub fn withdraw(env: Env, balance: i64, amount: i64) -> i64 { + balance - amount + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Env}; + +#[contract] +pub struct Bank; + +#[contractimpl] +impl Bank { + // The guard makes the invariant provable: the result is never negative. + #[invariant = "balance >= 0"] + pub fn withdraw(env: Env, balance: i64, amount: i64) -> i64 { + if amount > balance { + return balance; + } + balance - amount + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:H/A:H` +- **Base score:** 7.1 +- **Rating:** High + +A disproved invariant means a reachable state breaks a guarantee the contract relies on; reaching it may need specific inputs (higher complexity), but the impact on integrity and availability of the accounting logic is high. + +## How to fix + +1. Read the counterexample trace: it shows the exact inputs that violate the invariant. +2. Add the missing guard or bound (range check, saturating math, precondition) so the violating state is unreachable. +3. Re-run verification; the finding clears only when Z3 proves the invariant holds. +4. Strengthen or correct the invariant itself if it was mis-stated. + +## Related rules + +Related rules: [S003](S003.md), [S012](S012.md) + +## References + +- [Z3 theorem prover](https://github.com/Z3Prover/z3) +- [Soroban formal verification](https://soroban.stellar.org/docs/learn) +- [CWE-682: Incorrect Calculation](https://cwe.mitre.org/data/definitions/682.html) diff --git a/docs/rules/S012.md b/docs/rules/S012.md new file mode 100644 index 00000000..d8654f2a --- /dev/null +++ b/docs/rules/S012.md @@ -0,0 +1,93 @@ +# S012 — SEP-41 Interface Deviation + +- **Category:** token_interface +- **Severity:** Medium +- **Rule name:** `sep41_interface_deviation` + +## What it detects + +S012 verifies that token contracts implement the [SEP-41 (Stellar Token Standard)](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0041.md) interface correctly. For contracts identified as token candidates, it reports three deviation types: + +- **MissingFunction** — a required SEP-41 function (`allowance`, `approve`, `balance`, `transfer`, `transfer_from`, `burn`, `burn_from`, `decimals`, `name`, `symbol`) is absent. +- **SignatureMismatch** — a function exists but its parameter types, parameter order, or return type don't match the spec (e.g. `transfer` recipient must be `MuxedAddress`, `balance` must return `i128`). +- **AuthorizationMismatch** — a state-mutating function authorizes the wrong parameter or none at all (`approve`/`transfer`/`burn` must authorize `from`; `transfer_from`/`burn_from` must authorize `spender`). + +A contract is treated as a token candidate when it has ≥2 core functions, or ≥1 core function plus ≥2 metadata functions. + +## Why it matters + +SEP-41 is the contract that wallets, DEXes, and other protocols rely on to integrate with your token. A deviating signature or missing function means wallets reject your token or call it incorrectly; an authorization mismatch can let the wrong party move funds. Aligning exactly with the standard is what makes the token interoperable and safe to integrate. + +## Vulnerable example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env}; + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + // S012 SignatureMismatch: recipient should be MuxedAddress, not Address. + pub fn transfer(env: Env, from: Address, to: Address, amount: i128) { + from.require_auth(); + // ... + } + + // S012 AuthorizationMismatch: must authorize `spender`, not `from`. + pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { + from.require_auth(); + // ... + } +} +``` + +## Safe example + +```rust +#![no_std] +use soroban_sdk::{contract, contractimpl, Address, Env, MuxedAddress}; + +#[contract] +pub struct Token; + +#[contractimpl] +impl Token { + pub fn transfer(env: Env, from: Address, to: MuxedAddress, amount: i128) { + from.require_auth(); + // ... + } + + pub fn transfer_from(env: Env, spender: Address, from: Address, to: Address, amount: i128) { + // Authorize the spender, per SEP-41. + spender.require_auth(); + // ... verify and decrement allowance, then move the balance ... + } +} +``` + +## CVSS-style risk rating + +- **Vector:** `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:H/A:L` +- **Base score:** 5.9 +- **Rating:** Medium + +Most deviations cause integration failures; an authorization mismatch can additionally let the wrong principal move funds (high integrity impact) under specific conditions, keeping the realized risk at Medium. + +## How to fix + +1. Implement all 10 required SEP-41 functions with the exact parameter types, order, and return types. +2. Use `MuxedAddress` for the `transfer` recipient. +3. Authorize the correct principal in each mutating function: `from` for `approve`/`transfer`/`burn`, `spender` for `transfer_from`/`burn_from`. +4. Add integration tests against the standard via a generated `ContractClient`. + +## Related rules + +Related rules: [S001](S001.md), [S011](S011.md) + +## References + +- [SEP-41 specification](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0041.md) +- [Soroban token interface](https://soroban.stellar.org/docs/tokens/token-interface) +- [CWE-573: Improper Following of Specification](https://cwe.mitre.org/data/definitions/573.html) diff --git a/tooling/sanctifier-core/Cargo.toml b/tooling/sanctifier-core/Cargo.toml index bc7fee51..e4b657b6 100644 --- a/tooling/sanctifier-core/Cargo.toml +++ b/tooling/sanctifier-core/Cargo.toml @@ -37,6 +37,7 @@ chrono = { version = "0.4", features = ["serde"] } criterion = "0.5.1" insta = { version = "1.40", features = ["json", "redactions"] } serde_json = "1.0" +proptest = "1.4" [[bench]] name = "benchmark" diff --git a/tooling/sanctifier-core/src/input_validation.rs b/tooling/sanctifier-core/src/input_validation.rs index e880fbda..66c019e4 100644 --- a/tooling/sanctifier-core/src/input_validation.rs +++ b/tooling/sanctifier-core/src/input_validation.rs @@ -26,6 +26,17 @@ pub const MAX_SOURCE_BYTES: usize = 10 * 1024 * 1024; /// Minimum accepted source size (1 byte). pub const MIN_SOURCE_BYTES: usize = 1; +/// Maximum accepted delimiter nesting depth. +/// +/// The analysis engine (and `syn` itself) parses and walks the AST with +/// recursive descent. Pathologically deep delimiter nesting — e.g. a few +/// hundred unmatched `(((((…` — drives that recursion deep enough to overflow +/// the thread stack and abort the process, a classic denial-of-service vector. +/// Legitimate Soroban contracts never nest delimiters more than a few dozen +/// levels, so we reject anything past this bound up front, turning a hard crash +/// into a graceful validation error. +pub const MAX_NESTING_DEPTH: usize = 256; + /// Structured validation error returned by every guard. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ValidationError { @@ -142,6 +153,46 @@ pub fn validate_path(path: &str) -> Result<(), ValidationError> { pub fn validate_source_all(source: &str) -> Result<(), ValidationError> { validate_source_size(source)?; validate_no_null_bytes(source)?; + validate_nesting_depth(source)?; + Ok(()) +} + +/// Validate that delimiter nesting stays within [`MAX_NESTING_DEPTH`]. +/// +/// Counts the running open-delimiter depth across `(`, `[`, and `{`. This is a +/// conservative, O(n) guard: it also counts delimiters inside string and +/// comment tokens, which only makes it stricter, never looser. It exists purely +/// to stop deeply nested input from overflowing the recursive parser/analyzer +/// stack before `syn` ever sees it. +/// +/// # Errors +/// - `EXCESSIVE_NESTING` — nesting depth exceeds [`MAX_NESTING_DEPTH`]. +pub fn validate_nesting_depth(source: &str) -> Result<(), ValidationError> { + let mut depth: usize = 0; + let mut max_depth: usize = 0; + for byte in source.bytes() { + match byte { + b'(' | b'[' | b'{' => { + depth += 1; + if depth > max_depth { + max_depth = depth; + } + } + b')' | b']' | b'}' => { + depth = depth.saturating_sub(1); + } + _ => {} + } + } + if max_depth > MAX_NESTING_DEPTH { + return Err(ValidationError { + code: "EXCESSIVE_NESTING", + message: format!( + "Source nests delimiters {max_depth} levels deep; maximum allowed is {MAX_NESTING_DEPTH}. \ + This is rejected to prevent recursive-descent stack exhaustion." + ), + }); + } Ok(()) } diff --git a/tooling/sanctifier-core/src/lib.rs b/tooling/sanctifier-core/src/lib.rs index 2fe5ecda..dc3d70b5 100644 --- a/tooling/sanctifier-core/src/lib.rs +++ b/tooling/sanctifier-core/src/lib.rs @@ -410,33 +410,6 @@ impl std::fmt::Display for CustomRuleValidationError { } } -/// Project-level configuration loaded from `.sanctify.toml`. -#[derive(Debug, Serialize, Deserialize, Clone)] -pub struct SanctifyConfig { - /// Paths to skip during directory walking. - #[serde(default = "default_ignore_paths")] - pub ignore_paths: Vec, - /// Names of enabled built-in rules. - #[serde(default = "default_enabled_rules")] - pub enabled_rules: Vec, - /// Ledger-entry size limit in bytes. - #[serde(default = "default_ledger_limit")] - pub ledger_limit: usize, - /// Fraction of `ledger_limit` at which an *approaching* warning fires. - #[serde(default = "default_approaching_threshold")] - pub approaching_threshold: f64, - /// When `true`, use a tighter threshold for size warnings. - #[serde(default)] - pub strict_mode: bool, - /// User-defined regex rules. - #[serde(default)] - pub custom_rules: Vec, -} - -fn default_ignore_paths() -> Vec { - vec!["target".to_string(), ".git".to_string()] -} - #[derive(Debug, Serialize, Clone)] pub struct GasEstimation { pub function_name: String, @@ -517,6 +490,7 @@ impl Analyzer { rule_name: rule.name.clone(), line: line_no + 1, snippet: line.trim().to_string(), + severity: rule.severity, }); } } @@ -723,11 +697,6 @@ impl Analyzer { visitor.collisions } - pub fn scan_events(&self, _source: &str) -> Vec { - // Event scanning is not implemented in the current core engine. - Vec::new() - } - pub fn scan_unhandled_results(&self, source: &str) -> Vec { with_panic_guard(|| { self.run_rule(source, "unhandled_result") @@ -893,14 +862,13 @@ impl Analyzer { if !issue_locations.contains(&issue_key) { issue_locations.insert(issue_key); issues.push(EventIssue { - function_name: "unknown".to_string(), // scan_events_impl is regex-based, function context is limited event_name: event_name.clone(), - issue_type: EventIssueType::InconsistentSchema, + issue_type: "inconsistent_schema".to_string(), + location: location.clone(), message: format!( "Event '{}' has inconsistent topic count. Previous: {}, Current: {}", event_name, prev_count, topic_count ), - location: location.clone(), }); } } @@ -919,11 +887,10 @@ impl Analyzer { if !issue_locations.contains(&issue_key) { issue_locations.insert(issue_key); issues.push(EventIssue { - function_name: "unknown".to_string(), event_name, - issue_type: EventIssueType::OptimizableTopic, - message: "Consider using symbol_short! for short topic names to save gas.".to_string(), + issue_type: "optimizable_topic".to_string(), location: format!("line {}", line_num + 1), + message: "Consider using symbol_short! for short topic names to save gas.".to_string(), }); } } @@ -1002,73 +969,6 @@ impl Analyzer { suggestions: Vec::new(), }; - /// Validate custom rules before executing them (S007 UX improvement). - /// - /// Returns a list of [`CustomRuleValidationError`] for every rule whose - /// regex pattern fails to compile or whose name is empty. An empty return - /// value means all rules are ready to run. Call this once at startup so - /// broken configuration is surfaced to the user with a clear message rather - /// than silently dropped during analysis. - /// - /// # Example - /// ```rust,ignore - /// let errors = analyzer.validate_custom_rules(&config.custom_rules); - /// if !errors.is_empty() { - /// for e in &errors { eprintln!("Config error: {e}"); } - /// std::process::exit(1); - /// } - /// ``` - pub fn validate_custom_rules( - &self, - rules: &[CustomRule], - ) -> Vec { - use regex::Regex; - let mut errors = Vec::new(); - for rule in rules { - if rule.name.trim().is_empty() { - errors.push(CustomRuleValidationError { - rule_name: "".to_string(), - message: "rule name must not be empty".to_string(), - }); - } - if rule.pattern.is_empty() { - errors.push(CustomRuleValidationError { - rule_name: rule.name.clone(), - message: "pattern must not be empty — use a non-empty regex string".to_string(), - }); - } else if let Err(e) = Regex::new(&rule.pattern) { - errors.push(CustomRuleValidationError { - rule_name: rule.name.clone(), - message: format!("invalid regex pattern '{}': {}", rule.pattern, e), - }); - } - } - errors - } - - /// Run regex-based custom rules from config. Returns matches with line and snippet. - /// - /// Rules with invalid regex patterns are skipped silently. For upfront - /// validation that surfaces configuration errors, call - /// [`Analyzer::validate_custom_rules`] first. - pub fn analyze_custom_rules(&self, source: &str, rules: &[CustomRule]) -> Vec { - use regex::Regex; - - let mut matches = Vec::new(); - for rule in rules { - let re = match Regex::new(&rule.pattern) { - Ok(r) => r, - Err(_) => continue, - }; - for (line_no, line) in source.lines().enumerate() { - let line_num = line_no + 1; - if re.find(line).is_some() { - matches.push(CustomRuleMatch { - rule_name: rule.name.clone(), - line: line_num, - snippet: line.trim().to_string(), - severity: rule.severity, - }); // Collect #[contracttype] storage types for item in &file.items { if let Item::Struct(s) = item { @@ -1175,6 +1075,50 @@ impl Analyzer { report } + /// Validate custom rules before executing them (S007 UX improvement). + /// + /// Returns a list of [`CustomRuleValidationError`] for every rule whose + /// regex pattern fails to compile or whose name is empty. An empty return + /// value means all rules are ready to run. Call this once at startup so + /// broken configuration is surfaced to the user with a clear message rather + /// than silently dropped during analysis. + /// + /// # Example + /// ```rust,ignore + /// let errors = analyzer.validate_custom_rules(&config.custom_rules); + /// if !errors.is_empty() { + /// for e in &errors { eprintln!("Config error: {e}"); } + /// std::process::exit(1); + /// } + /// ``` + pub fn validate_custom_rules( + &self, + rules: &[CustomRule], + ) -> Vec { + use regex::Regex; + let mut errors = Vec::new(); + for rule in rules { + if rule.name.trim().is_empty() { + errors.push(CustomRuleValidationError { + rule_name: "".to_string(), + message: "rule name must not be empty".to_string(), + }); + } + if rule.pattern.is_empty() { + errors.push(CustomRuleValidationError { + rule_name: rule.name.clone(), + message: "pattern must not be empty — use a non-empty regex string".to_string(), + }); + } else if let Err(e) = Regex::new(&rule.pattern) { + errors.push(CustomRuleValidationError { + rule_name: rule.name.clone(), + message: format!("invalid regex pattern '{}': {}", rule.pattern, e), + }); + } + } + errors + } + fn fn_has_auth(&self, block: &syn::Block) -> bool { let mut has = false; self.check_fn_body(block, &mut false, &mut has); diff --git a/tooling/sanctifier-core/tests/proptest_analysis.rs b/tooling/sanctifier-core/tests/proptest_analysis.rs new file mode 100644 index 00000000..2987b2e9 --- /dev/null +++ b/tooling/sanctifier-core/tests/proptest_analysis.rs @@ -0,0 +1,311 @@ +//! Property-based tests for the analysis engine. +//! +//! # The property +//! +//! > The analysis engine must never panic — on **any** input, valid Rust or +//! > not. For every input string, running the full rule set returns a result +//! > that is either `Ok(findings)` or `Err(parse_error)`; it never aborts the +//! > process, never overflows the stack, and never runs out of memory. +//! +//! This is the contract real users depend on: Sanctifier is run over arbitrary, +//! often half-written or machine-generated contract source, and a single panic +//! in a rule would take down an editor's LSP session or a CI run. +//! +//! # What this generates +//! +//! Three input distributions, mixed together: +//! +//! 1. **Structured Soroban source** — randomly assembled `#[contractimpl]` +//! blocks whose bodies are built from fragments the rules specifically look +//! for (`require_auth`, raw arithmetic, `unwrap`, storage writes, casts, +//! `panic!`, …). These mostly parse, so they exercise rule *logic*. +//! 2. **Random Rust-ish snippets** — free-form statements and items that may or +//! may not parse. +//! 3. **Arbitrary text** — fully random Unicode (including control characters +//! and embedded NULs), to fuzz the parser/validator boundary. +//! +//! # Crash regressions +//! +//! Any input proptest discovers that makes the engine panic is persisted by +//! proptest to `tests/proptest-regressions/` and is replayed on every +//! subsequent run. A curated seed corpus of historically tricky inputs lives in +//! [`regression_corpus`] and is also asserted panic-free. +//! +//! # Tuning +//! +//! The case count defaults to 10,000 and can be overridden with the +//! `PROPTEST_CASES` environment variable (used by the dedicated, time-boxed CI +//! job). + +use std::panic::{catch_unwind, AssertUnwindSafe}; + +use proptest::prelude::*; + +use sanctifier_core::parser::{self, ParseError}; +use sanctifier_core::rules::RuleRegistry; +use sanctifier_core::{Analyzer, SanctifyConfig}; + +/// Default number of generated cases. Overridable via `PROPTEST_CASES`. +fn case_count() -> u32 { + std::env::var("PROPTEST_CASES") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(10_000) +} + +/// Run the **whole** analysis engine over `source`. +/// +/// Returns: +/// - `Ok(finding_count)` when the source parses and the rules run. +/// - `Err(parse_error)` when the validator or parser rejects the input. +/// +/// It must do one or the other for every possible input — never panic. The +/// rule registry (`run_all`) is intentionally invoked *without* a panic guard +/// so that a panicking rule surfaces here as a test failure rather than being +/// silently swallowed. +fn run_engine(source: &str) -> Result { + // Validate and parse FIRST. This is both the contract (Ok/Err split) and a + // safety gate: `parse_source` runs the input-validation guards (size, NUL + // bytes, and delimiter-nesting depth) before any recursive rule walks the + // source, so pathologically deep input is rejected here instead of + // overflowing the parser/analyzer stack. + parser::parse_source(source)?; + + let registry = RuleRegistry::with_default_rules(); + let analyzer = Analyzer::new(SanctifyConfig::default()); + + // `run_all` runs the full built-in rule registry (every S0xx syntactic + // rule). `verify_sep41_interface` covers the SEP-41 analysis that lives + // outside the registry, so the property exercises the whole rule surface. + let mut findings = registry.run_all(source).len(); + findings += analyzer.verify_sep41_interface(source).issues.len(); + + Ok(findings) +} + +/// Assert that analyzing `source` does not panic, returning a short label of +/// which arm of the contract was taken (for diagnostics on shrink). +fn assert_no_panic(source: &str) -> Result<&'static str, String> { + let result = catch_unwind(AssertUnwindSafe(|| run_engine(source))); + match result { + Ok(Ok(_)) => Ok("ok(findings)"), + Ok(Err(_)) => Ok("err(parse_error)"), + Err(_) => Err(format!( + "analysis engine PANICKED on input ({} bytes): {:?}", + source.len(), + truncate(source, 400), + )), + } +} + +fn truncate(s: &str, max: usize) -> String { + if s.len() <= max { + return s.to_string(); + } + let mut end = max; + while end > 0 && !s.is_char_boundary(end) { + end -= 1; + } + format!("{}…", &s[..end]) +} + +// ── Generators ────────────────────────────────────────────────────────────── + +/// A short lowercase identifier. +fn arb_ident() -> impl Strategy { + proptest::string::string_regex("[a-z][a-z0-9_]{0,7}").expect("valid regex") +} + +/// A Soroban-flavored type name. +fn arb_type() -> impl Strategy { + prop_oneof![ + Just("i128"), + Just("u32"), + Just("i64"), + Just("u64"), + Just("u128"), + Just("bool"), + Just("Address"), + Just("MuxedAddress"), + Just("String"), + Just("Bytes"), + Just("Env"), + ] +} + +/// A single statement, drawn from fragments that the rules actively scan for. +fn arb_stmt() -> impl Strategy { + prop_oneof![ + Just("from.require_auth();".to_string()), + Just("spender.require_auth_for_args(().into_val(&env));".to_string()), + Just("let s = a + b;".to_string()), + Just("let d = a - b;".to_string()), + Just("let m = a * b;".to_string()), + Just("let q = a / b;".to_string()), + Just("env.storage().persistent().set(&key, &value);".to_string()), + Just("env.storage().instance().set(&key, &value);".to_string()), + Just("let t = env.ledger().timestamp();".to_string()), + Just("let narrowed = wide as u32;".to_string()), + Just("result.unwrap();".to_string()), + Just("maybe.expect(\"must exist\");".to_string()), + Just("panic!(\"boom\");".to_string()), + Just("client.invoke_contract(&cid, &sym, args);".to_string()), + Just("env.events().publish((topic,), data);".to_string()), + Just("for i in 0..n { total += i; }".to_string()), + arb_ident().prop_map(|i| format!("let {i} = 0i128;")), + (arb_ident(), arb_type()).prop_map(|(i, t)| format!("let {i}: {t} = Default::default();")), + ] +} + +/// A public method with random params and a random body. +fn arb_fn() -> impl Strategy { + ( + arb_ident(), + prop::collection::vec((arb_ident(), arb_type()), 0..4), + prop::collection::vec(arb_stmt(), 0..5), + prop::option::of(arb_type()), + ) + .prop_map(|(name, params, body, ret)| { + let params: Vec = std::iter::once("env: Env".to_string()) + .chain(params.into_iter().map(|(n, t)| format!("{n}: {t}"))) + .collect(); + let ret = ret.map(|t| format!(" -> {t}")).unwrap_or_default(); + let body = body.join("\n "); + format!(" pub fn {name}({}){ret} {{\n {body}\n }}", params.join(", ")) + }) +} + +/// A complete, mostly-parseable Soroban contract. +fn arb_contract() -> impl Strategy { + prop::collection::vec(arb_fn(), 1..4).prop_map(|fns| { + format!( + "#![no_std]\nuse soroban_sdk::{{contract, contractimpl, Address, Env, String}};\n\ + #[contract]\npub struct C;\n#[contractimpl]\nimpl C {{\n{}\n}}\n", + fns.join("\n") + ) + }) +} + +/// Free-form Rust-ish snippets (often invalid). +fn arb_snippet() -> impl Strategy { + prop::collection::vec(arb_stmt(), 0..12).prop_map(|stmts| stmts.join("\n")) +} + +/// Fully arbitrary text, including control characters and embedded NULs. +fn arb_garbage() -> impl Strategy { + prop::collection::vec(any::(), 0..600).prop_map(|cs| cs.into_iter().collect()) +} + +/// The full mixed input distribution. +fn arb_source() -> impl Strategy { + prop_oneof![ + 6 => arb_contract(), + 3 => arb_snippet(), + 2 => arb_garbage(), + 1 => any::>().prop_map(|b| String::from_utf8_lossy(&b).into_owned()), + ] +} + +proptest! { + #![proptest_config(ProptestConfig { + cases: case_count(), + max_shrink_iters: 2_000, + .. ProptestConfig::default() + })] + + /// The engine never panics on any generated input. This is the headline + /// property; it runs the full `PROPTEST_CASES` budget (10,000 by default). + #[test] + fn engine_never_panics(source in arb_source()) { + match assert_no_panic(&source) { + Ok(_) => {}, + Err(message) => prop_assert!(false, "{message}"), + } + } +} + +proptest! { + // The Ok/Err invariant is cheaper to establish and does not need the full + // budget, so cap it to keep the suite inside the CI time box. + #![proptest_config(ProptestConfig { + cases: case_count().min(1_000), + .. ProptestConfig::default() + })] + + /// Whenever the source parses cleanly, analysis yields `Ok(findings)`; + /// whenever it does not, analysis yields `Err(parse_error)`. The two are + /// mutually exclusive and exhaustive. + #[test] + fn ok_xor_parse_error(source in arb_source()) { + let parses = parser::parse_source(&source).is_ok(); + let outcome = catch_unwind(AssertUnwindSafe(|| run_engine(&source))); + prop_assert!(outcome.is_ok(), "engine panicked"); + let outcome = outcome.unwrap(); + prop_assert_eq!(parses, outcome.is_ok()); + } +} + +// ── Regression corpus ──────────────────────────────────────────────────────── +// +// Curated inputs that have historically stressed the parser or rules. Any crash +// input discovered by proptest should be distilled to its essence and added +// here (in addition to proptest's own `tests/proptest-regressions/` artifacts), +// so the fix is locked in by a fast, deterministic test. + +/// Inputs known to probe edge cases: empty, whitespace, control characters, +/// deeply nested delimiters, oversized tokens, unicode identifiers, and partial +/// Soroban constructs. +const REGRESSION_CORPUS: &[&str] = &[ + "", + " ", + "\n\t\r", + "\0", + "fn", + "}}}}}}}}}}", + "{{{{{{{{{{", + "impl", + "#[contractimpl]", + "#[contractimpl] impl C {}", + "pub fn transfer(env: Env, from: Address) { from.require_auth(); }", + "impl C { pub fn f(env: Env) -> i128 { a - b } }", + "let x = 9999999999999999999999999999999999999999;", + "fn f() { let y = i128::MAX as u8; }", + "//\u{0}comment with nul", + "🦀 contract 🦀", + "impl C { pub fn f() { f().f().f().f().f().f(); } }", +]; + +#[test] +fn regression_corpus_never_panics() { + for (i, input) in REGRESSION_CORPUS.iter().enumerate() { + assert_no_panic(input) + .unwrap_or_else(|message| panic!("regression #{i} reintroduced a crash: {message}")); + } + + // DISCOVERED BY PROPTEST: a deeply nested expression used to overflow the + // recursive parser/analyzer stack and abort the process (SIGABRT). It is now + // rejected by the `EXCESSIVE_NESTING` validation guard, so analysis returns + // a graceful `Err(parse_error)` instead of crashing. + let nested = format!("fn f() {{ {} }}", "(".repeat(500)); + assert_eq!( + assert_no_panic(&nested), + Ok("err(parse_error)"), + "deep nesting must be rejected gracefully, not crash the process" + ); + + // A long but shallow token stream (many independent statements) must be + // processed without crashing. + let long_body = "let v = 1i128;\n".repeat(3_000); + let long = format!("fn f() {{ {long_body} }}"); + assert_no_panic(&long).expect("long flat input must be handled, not crash"); +} + +/// Sanity check that the harness actually distinguishes the two contract arms. +#[test] +fn contract_arms_are_reachable() { + assert_eq!( + assert_no_panic("pub fn f(env: Env) -> i128 { 0 }"), + Ok("ok(findings)") + ); + assert_eq!(assert_no_panic("this is not rust {{{"), Ok("err(parse_error)")); +}