Skip to content

Fix: 2 issues (#914, #915) - #935

Open
wendyamoni-creator wants to merge 1 commit into
SorobanGuard:mainfrom
wendyamoni-creator:simple/914-915
Open

wendyamoni-creator wants to merge 1 commit into
SorobanGuard:mainfrom
wendyamoni-creator:simple/914-915

Conversation

@wendyamoni-creator

Copy link
Copy Markdown
Contributor

Closes #914
Closes #915

Issues

#914 analyzer: scanning a single .rs file does not follow out-of-line mod foo; declarations

Problem

scan path/to/lib.rs parses only that file. A contract split across mod storage; / mod admin; files has its submodules silently skipped, while scan path/to/dir finds them because it walks the directory. The same contract therefore yields different findings depending on how it is invoked, with no warning.

Expected

When the target is a single file, resolve mod name; declarations (name.rs or name/mod.rs, honouring #[path = "..."]) relative to it and scan them too, with cycle protection and de-duplication against files already scanned.

Acceptance criteria

  • Missing module files produce a warning, not an abort.
  • Files reached this way respect --exclude and generated-file skipping.
  • Test using a temp crate with a nested module tree.
  • Behaviour documented in docs/architecture.md.

#915 checks: add unchecked-negation-or-abs — -amount / .abs() on i128 panics at MIN

Problem

In Soroban contracts, -x and x.abs() on a signed integer overflow when x == i128::MIN. The release profile commonly sets overflow-checks = true, so this becomes a trap the caller can trigger by passing i128::MIN as an amount. No existing check covers unary negation or abs; unchecked-arithmetic only looks at binary operators.

Proposal

Add a Medium-severity check, unchecked-negation-or-abs, that flags unary - and .abs() applied to a non-literal value inside #[contractimpl] functions (excluding #[cfg(test)]). Do not flag when the operand is guarded by a comparison against MIN/0, or uses checked_neg, checked_abs, unsigned_abs or saturating_neg.

Acceptance criteria

  • Registered once via the central registry; docs section in docs/checks.md; rule_url set.
  • test-contracts/negation-safe and negation-vulnerable fixtures plus fixture_scans.rs assertions.
  • Entry added to examples/all-findings; registry_consistency.rs passes.

Summary

For #914, factor module discovery into a helper shared by scan_directory, scan_directory_with_checks, and scan_files; keep the visited set keyed by canonical path. For #915, reuse contractimpl_functions_excluding_test and the existing visitor patterns in overflow.rs; ensure the check name is unique and that rule_url follows the same format as other checks. The fixture crates need a minimal Cargo.toml matching existing test-contracts/* crates if the workspace requires it.

Changes

  • Extend the single-file scan path so that when the scan root is a .rs file, out-of-line mod name; declarations are resolved recursively relative to the declaring file (name.rs or name/mod.rs, honouring #[path = "..."]). Maintain a visited set for cycle protection and de-duplication against already-scanned files, apply the same --exclude and generated-file skipping as directory walking, and emit warnings (not errors) for missing module files. Ensure scan_directory, scan_directory_with_checks, and scan_files all use this discovery so a single-file scan matches a directory scan. (crates/analyzer/src/lib.rs)
  • Add a test using a temporary crate with a nested module tree: create src/lib.rs containing mod storage; and mod admin;, with src/storage.rs and src/admin/mod.rs containing vulnerable contract code. Assert that scanning only src/lib.rs produces the same findings as scanning the directory, that missing module files produce a warning rather than an abort, and that excluded/generated module files are skipped. (crates/analyzer/tests/scan_files_api.rs)
  • Document the single-file scan behaviour: when the scan target is a single .rs file, out-of-line mod name; declarations are followed recursively (name.rs / name/mod.rs, honouring #[path]), with cycle protection, de-duplication, exclude/generated-file filtering, and warnings for missing module files, so single-file and directory scans agree. (docs/architecture.md)
  • Implement the unchecked-negation-or-abs check (Medium severity): walk #[contractimpl] functions excluding #[cfg(test)], flag unary - and .abs() applied to non-literal signed-integer operands, and suppress findings when the operand is guarded by a comparison against MIN/0 or when checked_neg, checked_abs, unsigned_abs, or saturating_neg is used. Set rule_url and include unit tests for vulnerable and safe forms. (crates/checks/src/overflow.rs)
  • Register the new check once via the central registry: add pub mod negation; (or re-export from overflow), export UncheckedNegationOrAbsCheck, and add Box::new(UncheckedNegationOrAbsCheck) to all_checks_base() so default_checks() and default_checks_with_config() include it exactly once. (crates/checks/src/lib.rs)
  • Add the unchecked-negation-or-abs entry to the FIXTURES table mapping it to the negation fixture base so registry_consistency.rs requires test-contracts/negation-vulnerable and test-contracts/negation-safe. (crates/checks/tests/registry_consistency.rs)
  • Add negation_fixtures() calling assert_fixture_pair("negation", "unchecked-negation-or-abs") so the vulnerable fixture must produce the finding and the safe fixture must not. (crates/cli/tests/fixture_scans.rs)
  • Add a ## unchecked-negation-or-abs (Medium) section documenting what it detects (unary - and .abs() on non-literal signed integers in #[contractimpl] methods), why i128::MIN negation/abs traps under overflow-checks = true, the guarded/checked/saturating/unsigned exceptions, limitations, and the test-contracts/negation-vulnerable/ and test-contracts/negation-safe/ fixtures. (docs/checks.md)
  • Add a method to examples/all-findings/src/lib.rs with a // Triggers unchecked-negation-or-abs`` comment that negates or calls .abs() on a non-literal signed integer inside the `#[contractimpl]` block, so the registry consistency test finds the trigger. (examples/all-findings/src/lib.rs)
  • Create the vulnerable fixture crate test-contracts/negation-vulnerable/src/lib.rs containing a #[contractimpl] method that applies unary - and .abs() to a non-literal i128 amount without a MIN/0 guard, so the check fires. (test-contracts/negation-vulnerable/src/lib.rs)
  • Create the safe fixture crate test-contracts/negation-safe/src/lib.rs containing #[contractimpl] methods that guard against i128::MIN/0 before negating or use checked_neg, checked_abs, unsigned_abs, or saturating_neg, so the check does not fire. (test-contracts/negation-safe/src/lib.rs)

Co-authored-by: Simple Agent <agent@simple.dev>
@drips-wave

drips-wave Bot commented Oct 22, 2026

Copy link
Copy Markdown

@wendyamoni-creator Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant