diff --git a/packages/rules/soroban/src/collections/map-ops-check.ts b/packages/rules/soroban/src/collections/map-ops-check.ts new file mode 100644 index 0000000..74dbc68 --- /dev/null +++ b/packages/rules/soroban/src/collections/map-ops-check.ts @@ -0,0 +1,126 @@ +/** + * Rule: Detect Inefficient Soroban Map Operations + * + * Repeated lookups and unnecessary map mutations increase contract resource + * consumption in Soroban. Each host-object map access is metered. + * + * Issue: #768 + */ + +export interface MapOpsWarning { + line: number; + column?: number; + patternType: 'repeated-lookup' | 'unnecessary-update' | 'avoidable-traversal'; + key?: string; + message: string; + suggestion: string; +} + +export class SorobanMapOpsCheckRule { + public static readonly RULE_ID = 'soroban-inefficient-map-ops'; + + /** Map get patterns – each lookup is a metered host call. */ + private static readonly LOOKUP_PATTERNS = [ + '.get(', + '.contains_key(', + '.try_get(', + ]; + + /** Mutation patterns that write the same entry without guard. */ + private static readonly UPDATE_PATTERNS = [ + '.set(', + '.insert(', + '.put(', + ]; + + /** Full traversal of a Map is expensive due to host-object iteration cost. */ + private static readonly TRAVERSAL_PATTERNS = [ + '.iter()', + '.keys()', + '.values()', + '.into_iter()', + ]; + + public analyze(sourceCode: string): MapOpsWarning[] { + const warnings: MapOpsWarning[] = []; + const lines = sourceCode.split('\n'); + + // Track lookup keys per function to detect repeated lookups for the same key + const lookupKeysPerFunction: Map> = new Map(); + let currentFunction = ''; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const lineNum = i + 1; + + // Track function scope + const fnMatch = line.match(/\bfn\s+([a-zA-Z0-9_]+)\s*\(/); + if (fnMatch) { + currentFunction = fnMatch[1]; + if (!lookupKeysPerFunction.has(currentFunction)) { + lookupKeysPerFunction.set(currentFunction, new Map()); + } + } + + // 1. Repeated lookup for the same key + for (const pattern of SorobanMapOpsCheckRule.LOOKUP_PATTERNS) { + if (line.includes(pattern)) { + // Extract the key argument (rough heuristic: text inside first parens) + const keyMatch = line.match(/\.(?:get|contains_key|try_get)\(([^)]+)\)/); + const key = keyMatch ? keyMatch[1].trim() : 'unknown'; + + const fnKeys = lookupKeysPerFunction.get(currentFunction) ?? new Map(); + const count = (fnKeys.get(key) ?? 0) + 1; + fnKeys.set(key, count); + lookupKeysPerFunction.set(currentFunction, fnKeys); + + if (count > 1) { + warnings.push({ + line: lineNum, + patternType: 'repeated-lookup', + key, + message: `Key '${key}' is looked up more than once in function '${currentFunction}'. Each map lookup is a metered host call.`, + suggestion: `Cache the result of the first lookup in a local variable and reuse it instead of calling .get(${key}) again.`, + }); + } + break; + } + } + + // 2. Unnecessary update – set() called without a prior guard/condition check + for (const pattern of SorobanMapOpsCheckRule.UPDATE_PATTERNS) { + if (line.includes(pattern)) { + // Warn if the set/insert is NOT preceded by a contains_key check on the same line + // or the immediately preceding lines (simple heuristic). + const prevLines = lines.slice(Math.max(0, i - 3), i).join('\n'); + if (!prevLines.includes('contains_key') && !prevLines.includes('if ') && !prevLines.includes('match ')) { + warnings.push({ + line: lineNum, + patternType: 'unnecessary-update', + message: `Unconditional map update ('${pattern.trim()}') without a prior existence check may overwrite existing values unnecessarily, wasting metered write budget.`, + suggestion: + 'Guard the update with a .contains_key() check or use an entry-based pattern to avoid redundant writes.', + }); + } + break; + } + } + + // 3. Avoidable full traversal + for (const pattern of SorobanMapOpsCheckRule.TRAVERSAL_PATTERNS) { + if (line.includes(pattern)) { + warnings.push({ + line: lineNum, + patternType: 'avoidable-traversal', + message: `Full map traversal ('${pattern.trim()}') in function '${currentFunction}' is expensive. Iterating over a Soroban Map enumerates all host objects.`, + suggestion: + 'Access only the specific keys you need via direct .get() calls, or restructure data to avoid full traversal.', + }); + break; + } + } + } + + return warnings; + } +} diff --git a/packages/rules/soroban/src/collections/vector-ops-check.ts b/packages/rules/soroban/src/collections/vector-ops-check.ts new file mode 100644 index 0000000..5351256 --- /dev/null +++ b/packages/rules/soroban/src/collections/vector-ops-check.ts @@ -0,0 +1,123 @@ +/** + * Rule: Detect Inefficient Soroban Vector Operations + * + * Repeated vector traversal, front insertion/removal, and unnecessary copying + * increase metered CPU and memory budget in Soroban contracts. + * + * Issue: #767 + */ + +export interface VectorOpsWarning { + line: number; + column?: number; + patternType: 'repeated-traversal' | 'inefficient-insertion' | 'unnecessary-copy'; + symbol?: string; + message: string; + suggestion: string; +} + +export class SorobanVectorOpsCheckRule { + public static readonly RULE_ID = 'soroban-inefficient-vector-ops'; + + /** + * Patterns that indicate a full vector traversal (O(n) scan). + * In Soroban, each host-object access is metered. + */ + private static readonly TRAVERSAL_PATTERNS = [ + '.iter()', + '.iter_mut()', + '.into_iter()', + '.for_each(', + 'for ', + ]; + + /** + * Insertion/removal at the front is O(n) for Vec – push_back / push_front + * differ in cost on Soroban host vectors. + */ + private static readonly INEFFICIENT_INSERT_PATTERNS = [ + '.push_front(', + '.insert(0,', + '.remove(0)', + ]; + + /** Unnecessary clone / copy of an entire vector. */ + private static readonly COPY_PATTERNS = [ + '.clone()', + 'Vec::from(', + '.to_vec()', + ]; + + public analyze(sourceCode: string): VectorOpsWarning[] { + const warnings: VectorOpsWarning[] = []; + const lines = sourceCode.split('\n'); + + // Track traversal counts per function to detect repeated traversals + const traversalCountPerFunction: Map = new Map(); + let currentFunction = ''; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const lineNum = i + 1; + + // Detect function boundaries (rough heuristic) + const fnMatch = line.match(/\bfn\s+([a-zA-Z0-9_]+)\s*\(/); + if (fnMatch) { + currentFunction = fnMatch[1]; + if (!traversalCountPerFunction.has(currentFunction)) { + traversalCountPerFunction.set(currentFunction, 0); + } + } + + // 1. Repeated vector traversal + for (const pattern of SorobanVectorOpsCheckRule.TRAVERSAL_PATTERNS) { + if (line.includes(pattern)) { + const count = (traversalCountPerFunction.get(currentFunction) ?? 0) + 1; + traversalCountPerFunction.set(currentFunction, count); + + if (count > 1) { + warnings.push({ + line: lineNum, + patternType: 'repeated-traversal', + symbol: currentFunction, + message: `Function '${currentFunction}' performs multiple vector traversals. Each traversal consumes metered Soroban CPU budget.`, + suggestion: + 'Combine traversals into a single pass or cache intermediate results in a local variable.', + }); + } + break; + } + } + + // 2. Inefficient insertion / removal + for (const pattern of SorobanVectorOpsCheckRule.INEFFICIENT_INSERT_PATTERNS) { + if (line.includes(pattern)) { + warnings.push({ + line: lineNum, + patternType: 'inefficient-insertion', + message: `Front insertion/removal ('${pattern.trim()}') on a vector is O(n) and shifts all elements, wasting metered CPU.`, + suggestion: + 'Use push_back() for appending, or consider a different data structure (e.g., a Map) if random access is needed.', + }); + break; + } + } + + // 3. Unnecessary copy + for (const pattern of SorobanVectorOpsCheckRule.COPY_PATTERNS) { + if (line.includes(pattern) && line.includes('Vec')) { + warnings.push({ + line: lineNum, + patternType: 'unnecessary-copy', + message: `Unnecessary vector copy detected ('${pattern.trim()}'). Cloning a Soroban Vec allocates a new host object and doubles memory budget consumption.`, + suggestion: + 'Pass a reference or slice instead of cloning the entire vector where ownership is not strictly required.', + }); + break; + } + } + } + + return warnings; + } +} diff --git a/packages/rules/soroban/src/index.ts b/packages/rules/soroban/src/index.ts index 77044e6..e01da97 100644 --- a/packages/rules/soroban/src/index.ts +++ b/packages/rules/soroban/src/index.ts @@ -1,5 +1,7 @@ export * from './storage-rent-check'; export * from './analyzer/wasm-inspector'; +export * from './collections/vector-ops-check'; +export * from './collections/map-ops-check'; export * from './analyzer/callgraph-analyzer'; export * from './analyzer/serialization-analyzer'; export * from './calls'; diff --git a/packages/rules/src/soroban/loop_cost_analyzer.rs b/packages/rules/src/soroban/loop_cost_analyzer.rs new file mode 100644 index 0000000..7b746d8 --- /dev/null +++ b/packages/rules/src/soroban/loop_cost_analyzer.rs @@ -0,0 +1,178 @@ +//! Rule: Soroban Loop Cost Analyzer +//! +//! Analyzes loops for expensive execution patterns that consume excess metered +//! Soroban resources. Detects: +//! +//! * Storage access inside loop bodies (`env.storage()` calls inside `for`/`while`/`loop`). +//! * Nested loops, which multiply execution cost. +//! * Cross-contract calls inside loops. +//! +//! Issue: #769 + +use crate::soroban::rule_engine::SorobanRule; +use crate::soroban::SorobanContract; +use crate::{RuleViolation, ViolationSeverity}; + +/// Keywords that open a loop body. +const LOOP_OPENERS: &[&str] = &["for ", "while ", "loop {"]; + +/// Patterns indicating a storage operation (metered host call). +const STORAGE_PATTERNS: &[&str] = &[ + "env.storage()", + ".persistent()", + ".temporary()", + ".instance()", +]; + +/// Patterns indicating a cross-contract call. +const CROSS_CONTRACT_PATTERNS: &[&str] = &[ + "invoke_contract", + "call(", + "Client::new(", + "client.invoke(", +]; + +/// Analyzes Soroban contract loops for cost-heavy patterns. +pub struct LoopCostAnalyzerRule { + enabled: bool, +} + +impl Default for LoopCostAnalyzerRule { + fn default() -> Self { + Self { enabled: true } + } +} + +impl SorobanRule for LoopCostAnalyzerRule { + fn id(&self) -> &str { + "soroban-loop-cost-analyzer" + } + + fn name(&self) -> &str { + "Loop Cost Analyzer" + } + + fn description(&self) -> &str { + "Detects expensive operations (storage access, nested loops, cross-contract calls) \ + inside Soroban loop bodies that multiply metered CPU and memory costs." + } + + fn severity(&self) -> ViolationSeverity { + ViolationSeverity::High + } + + fn is_enabled(&self) -> bool { + self.enabled + } + + fn set_enabled(&mut self, enabled: bool) { + self.enabled = enabled; + } + + fn apply(&self, contract: &SorobanContract) -> Vec { + let mut violations = Vec::new(); + let lines: Vec<&str> = contract.source.lines().collect(); + + // Track brace depth to understand when we enter/exit a loop body + let mut loop_depth: usize = 0; + let mut nested_loop_start: Option = None; + + for (i, line) in lines.iter().enumerate() { + let line_num = i + 1; + let trimmed = line.trim(); + + // Detect loop openers + let opens_loop = LOOP_OPENERS.iter().any(|p| trimmed.starts_with(p) || trimmed.contains(p)); + if opens_loop { + if loop_depth > 0 { + // Already inside a loop – this is a nested loop + if nested_loop_start.is_none() { + nested_loop_start = Some(line_num); + } + violations.push(RuleViolation { + rule_name: self.id().to_string(), + description: "Nested loop detected. Nested loops multiply execution cost \ + (O(n²) or worse) and can exhaust Soroban CPU budget." + .to_string(), + suggestion: + "Flatten the nested loop or precompute intermediate results outside \ + the outer loop to reduce total iterations." + .to_string(), + line_number: line_num, + column_number: 0, + variable_name: String::new(), + severity: ViolationSeverity::High, + }); + } + loop_depth += 1; + } + + // Track brace balance to detect loop end (rough heuristic) + let opens = trimmed.chars().filter(|&c| c == '{').count(); + let closes = trimmed.chars().filter(|&c| c == '}').count(); + // Only update depth on non-loop-opening lines to avoid double-counting + if !opens_loop { + // Net brace change adjusts loop tracking + } + if closes > opens && loop_depth > 0 { + let diff = closes - opens; + loop_depth = loop_depth.saturating_sub(diff); + if loop_depth == 0 { + nested_loop_start = None; + } + } + + // Inside a loop – check for expensive patterns + if loop_depth > 0 { + // Storage access inside loop + for pattern in STORAGE_PATTERNS { + if trimmed.contains(pattern) { + violations.push(RuleViolation { + rule_name: self.id().to_string(), + description: format!( + "Storage operation ('{}') inside a loop body. \ + Each storage call is a metered host operation; \ + calling it per iteration multiplies the cost linearly.", + pattern + ), + suggestion: + "Cache the storage value in a local variable before the loop \ + and write back once after the loop completes." + .to_string(), + line_number: line_num, + column_number: 0, + variable_name: String::new(), + severity: ViolationSeverity::High, + }); + break; + } + } + + // Cross-contract call inside loop + for pattern in CROSS_CONTRACT_PATTERNS { + if trimmed.contains(pattern) { + violations.push(RuleViolation { + rule_name: self.id().to_string(), + description: format!( + "Cross-contract call ('{}') detected inside a loop. \ + Each invocation consumes significant CPU and auth budget.", + pattern + ), + suggestion: + "Batch cross-contract operations outside the loop, or redesign \ + the contract interface to accept bulk inputs." + .to_string(), + line_number: line_num, + column_number: 0, + variable_name: String::new(), + severity: ViolationSeverity::Critical, + }); + break; + } + } + } + } + + violations + } +} diff --git a/packages/rules/src/soroban/mod.rs b/packages/rules/src/soroban/mod.rs index f3c2a3f..d2190c7 100644 --- a/packages/rules/src/soroban/mod.rs +++ b/packages/rules/src/soroban/mod.rs @@ -5,6 +5,11 @@ //! `#[contract]`, `#[contractimpl]`, and `#[contracttype]`. pub mod analyzer; +pub mod loop_cost_analyzer; +pub mod memory; +pub mod parser; +pub mod rule_engine; +pub mod unbounded_iteration; pub mod event_emission; pub mod inefficient_error_construction; pub mod memory; diff --git a/packages/rules/src/soroban/rule_engine.rs b/packages/rules/src/soroban/rule_engine.rs index bcc06e8..c888fce 100644 --- a/packages/rules/src/soroban/rule_engine.rs +++ b/packages/rules/src/soroban/rule_engine.rs @@ -3,6 +3,9 @@ //! This module provides a specialized rule engine for analyzing Soroban smart contracts //! with rules tailored to Soroban's unique characteristics and gas optimization patterns. +use crate::soroban::loop_cost_analyzer::LoopCostAnalyzerRule; +use crate::soroban::memory::InefficientBytesAllocationRule; +use crate::soroban::unbounded_iteration::UnboundedIterationRule; use crate::soroban::memory::{InefficientBytesAllocationRule, MemoryAllocationRule}; use crate::soroban::{ EventEmissionCostRule, InefficientErrorConstructionRule, UnnecessaryCloningRule, @@ -58,6 +61,8 @@ impl SorobanRuleEngine { .add_rule(AntiFrontRunningRule::default()) // #118 .add_rule(SecureRandomnessRule::default()) // #119 .add_rule(UpgradeVersionTrackingRule::default()) // #123 + .add_rule(LoopCostAnalyzerRule::default()) // #769 + .add_rule(UnboundedIterationRule::default()); // #770 .add_rule(UnnecessaryCloningRule::default()) // #775 .add_rule(MemoryAllocationRule::default()) // #776 .add_rule(InefficientErrorConstructionRule::default()) // #777 diff --git a/packages/rules/src/soroban/unbounded_iteration.rs b/packages/rules/src/soroban/unbounded_iteration.rs new file mode 100644 index 0000000..4ce8bac --- /dev/null +++ b/packages/rules/src/soroban/unbounded_iteration.rs @@ -0,0 +1,164 @@ +//! Rule: Detect Unbounded Soroban Iteration +//! +//! Loops whose iteration count cannot be statically bounded (e.g., iterating +//! over a collection whose size is controlled by user input or contract state) +//! can exhaust Soroban CPU/memory limits and cause transaction failure. +//! +//! This rule detects: +//! +//! * `for` loops over a range derived from a function parameter or storage value. +//! * `while` loops without an obvious constant upper bound. +//! * `loop` blocks with no early-exit guard on a bounded counter. +//! +//! Issue: #770 + +use crate::soroban::rule_engine::SorobanRule; +use crate::soroban::SorobanContract; +use crate::{RuleViolation, ViolationSeverity}; + +/// Patterns that suggest the loop bound comes from external / dynamic input. +const DYNAMIC_BOUND_PATTERNS: &[&str] = &[ + "env.storage()", + ".len()", + ".count()", + "params.", + "args.", + "input.", + "request.", +]; + +/// Patterns for `while` loops that lack a fixed upper bound. +const UNBOUNDED_WHILE_PATTERNS: &[&str] = &["while true", "while !done", "while running"]; + +/// Detects loops that cannot be statically bounded. +pub struct UnboundedIterationRule { + enabled: bool, +} + +impl Default for UnboundedIterationRule { + fn default() -> Self { + Self { enabled: true } + } +} + +impl SorobanRule for UnboundedIterationRule { + fn id(&self) -> &str { + "soroban-unbounded-iteration" + } + + fn name(&self) -> &str { + "Unbounded Soroban Iteration" + } + + fn description(&self) -> &str { + "Detects loops whose iteration count is not statically bounded. \ + Unbounded iteration can exhaust Soroban CPU/memory limits and \ + cause transaction failure or denial-of-service." + } + + fn severity(&self) -> ViolationSeverity { + ViolationSeverity::High + } + + fn is_enabled(&self) -> bool { + self.enabled + } + + fn set_enabled(&mut self, enabled: bool) { + self.enabled = enabled; + } + + fn apply(&self, contract: &SorobanContract) -> Vec { + let mut violations = Vec::new(); + let lines: Vec<&str> = contract.source.lines().collect(); + + for (i, line) in lines.iter().enumerate() { + let line_num = i + 1; + let trimmed = line.trim(); + + // --- `for` loop with a dynamic upper bound --- + if trimmed.starts_with("for ") || trimmed.contains(" for ") { + let has_dynamic_bound = DYNAMIC_BOUND_PATTERNS + .iter() + .any(|p| trimmed.contains(p)); + + if has_dynamic_bound { + violations.push(RuleViolation { + rule_name: self.id().to_string(), + description: + "For-loop iterates over a dynamically-sized collection or range. \ + The number of iterations is not statically bounded, which can \ + exhaust Soroban metered CPU budget." + .to_string(), + suggestion: + "Add an explicit upper-bound cap (e.g., `let safe_len = \ + coll.len().min(MAX_ITEMS);`) and assert inputs do not exceed \ + a safe limit before entering the loop." + .to_string(), + line_number: line_num, + column_number: 0, + variable_name: String::new(), + severity: ViolationSeverity::High, + }); + } + } + + // --- Infinite / unbounded `while` loops --- + for pattern in UNBOUNDED_WHILE_PATTERNS { + if trimmed.contains(pattern) { + violations.push(RuleViolation { + rule_name: self.id().to_string(), + description: format!( + "Potentially unbounded while-loop ('{}') detected. \ + Without a guaranteed termination condition this loop \ + may run until resources are exhausted.", + pattern + ), + suggestion: + "Replace the unbounded while with a for-loop over a capped range, \ + or add a hard iteration counter that breaks after a safe maximum." + .to_string(), + line_number: line_num, + column_number: 0, + variable_name: String::new(), + severity: ViolationSeverity::High, + }); + break; + } + } + + // --- Bare `loop {}` without an obvious bounded counter --- + if trimmed == "loop {" || trimmed == "loop{" { + // Look ahead a few lines for a break condition referencing a counter + let lookahead = lines + .get(i + 1..i.saturating_add(10).min(lines.len())) + .unwrap_or(&[]); + let has_bounded_break = lookahead.iter().any(|l| { + let t = l.trim(); + t.contains("break") && (t.contains(">=") || t.contains("==") || t.contains('>')) + }); + + if !has_bounded_break { + violations.push(RuleViolation { + rule_name: self.id().to_string(), + description: + "Bare `loop {}` block without an obvious bounded break condition. \ + If the exit condition depends on external state this loop \ + may consume all available Soroban CPU budget." + .to_string(), + suggestion: + "Add a counter variable that increments each iteration and breaks \ + when it exceeds a compile-time constant (e.g., `const MAX: u32 = 100`)." + .to_string(), + line_number: line_num, + column_number: 0, + variable_name: String::new(), + severity: ViolationSeverity::Medium, + }); + } + } + } + + violations + } +}