diff --git a/rules/optimization/memory/detect-expensive-memory-copies.ts b/rules/optimization/memory/detect-expensive-memory-copies.ts new file mode 100644 index 0000000..9a7ac5b --- /dev/null +++ b/rules/optimization/memory/detect-expensive-memory-copies.ts @@ -0,0 +1,105 @@ +export interface ExpensiveMemoryCopy { + type: string; + line: number; + description: string; + recommendation: string; +} + +export interface ExpensiveMemoryCopiesResult { + detected: boolean; + copies: ExpensiveMemoryCopy[]; + message: string; + suggestion: string; +} + +interface MemoryCopyPattern { + type: string; + pattern: RegExp; + description: string; + recommendation: string; +} + +const MEMORY_COPY_PATTERNS: MemoryCopyPattern[] = [ + { + type: "unnecessary-clone", + pattern: /\.clone\(\)/g, + description: + "`.clone()` creates a deep copy of the data, incurring gas cost for large or complex types.", + recommendation: + "Use references (&T) where possible instead of cloning. Clone only when an owned copy is strictly necessary.", + }, + { + type: "string-to-string-copy", + pattern: /String::from\(\s*\w+\s*\)|\.to_string\(\)|\.into\(\)/g, + description: + "String conversion or allocation creates a new heap allocation, which is expensive in Soroban contracts.", + recommendation: + "Use &str references or Symbol where possible. Avoid unnecessary String allocations in hot paths.", + }, + { + type: "vec-copy", + pattern: /\.to_vec\(\)|\.iter\(\).*\.collect\(\)/g, + description: + "Copying an entire Vec or similar collection creates a deep copy of all elements.", + recommendation: + "Use slice references (&[T]) instead of copying the entire Vec when read-only access is sufficient.", + }, + { + type: "large-struct-copy", + pattern: + /fn\s+\w+\s*\([^)]*(?:&self|self|&mut self)[^)]*\)\s*->\s*\w+\s*\{[^}]*\b(?:clone|to_owned)\b/g, + description: + "Functions returning owned copies of large structs force unnecessary memory duplication.", + recommendation: + "Return references where the lifetime allows, or use Cow (clone-on-write) for conditional copying.", + }, +]; + +function lineNumber(code: string, index: number): number { + let line = 1; + for (let i = 0; i < index; i++) { + if (code[i] === "\n") line++; + } + return line; +} + +export function detectExpensiveMemoryCopies( + code: string, +): ExpensiveMemoryCopiesResult { + const copies: ExpensiveMemoryCopy[] = []; + + for (const { + type, + pattern, + description, + recommendation, + } of MEMORY_COPY_PATTERNS) { + const re = new RegExp(pattern.source, "g"); + let match: RegExpExecArray | null; + while ((match = re.exec(code)) !== null) { + copies.push({ + type, + line: lineNumber(code, match.index), + description, + recommendation, + }); + } + } + + if (copies.length === 0) { + return { + detected: false, + copies: [], + message: "No expensive memory copies detected.", + suggestion: "", + }; + } + + return { + detected: true, + copies, + message: `Detected ${copies.length} expensive memory copy operation(s).`, + suggestion: + "Use references (&T) instead of cloning, prefer &str over String, and avoid copying large collections.", + }; +} diff --git a/rules/optimization/storage/detect-expensive-sload-operations.ts b/rules/optimization/storage/detect-expensive-sload-operations.ts new file mode 100644 index 0000000..00e0283 --- /dev/null +++ b/rules/optimization/storage/detect-expensive-sload-operations.ts @@ -0,0 +1,93 @@ +export interface ExpensiveSLOAD { + type: string; + line: number; + description: string; + recommendation: string; +} + +export interface ExpensiveSLOADResult { + detected: boolean; + sloads: ExpensiveSLOAD[]; + message: string; + suggestion: string; +} + +interface SLOADPattern { + type: string; + pattern: RegExp; + description: string; + recommendation: string; +} + +const SLOAD_PATTERNS: SLOADPattern[] = [ + { + type: "repeated-sload", + pattern: /\.get\s*\([^)]+\)\s*;\s*(?:.|\n)*?\.get\s*\(/g, + description: + "Repeated SLOAD operations for the same storage key within a function. Each SLOAD costs gas.", + recommendation: + "Cache storage reads in a local variable at the start of the function to avoid repeated SLOAD costs.", + }, + { + type: "sload-in-loop", + pattern: + /(?:for|while)\s*[^{]*\{[^}]*?(?:storage|env\.storage)\(\)\.(?:instance|persistent|temporary)\(\)\.get\s*\(/g, + description: + "SLOAD operation inside a loop. Each iteration reads from storage, significantly increasing gas costs.", + recommendation: + "Read storage values into a local variable before the loop and reference the cached value inside the loop body.", + }, + { + type: "unbatched-sload", + pattern: + /(?:storage|env\.storage)\(\)\.(?:instance|persistent|temporary)\(\)\.get\s*\([^)]+\)\s*;\s*(?:\s*\/\/[^\n]*\n)*\s*(?:storage|env\.storage)\(\)\.(?:instance|persistent|temporary)\(\)\.get/g, + description: + "Multiple unbatched SLOAD operations. Each access incurs individual gas cost.", + recommendation: + "Batch related storage reads or cache them in local variables to reduce gas consumption.", + }, +]; + +function lineNumber(code: string, index: number): number { + let line = 1; + for (let i = 0; i < index; i++) { + if (code[i] === "\n") line++; + } + return line; +} + +export function detectExpensiveSLOADOperations( + code: string, +): ExpensiveSLOADResult { + const sloads: ExpensiveSLOAD[] = []; + + for (const { type, pattern, description, recommendation } of SLOAD_PATTERNS) { + const re = new RegExp(pattern.source, "g"); + let match: RegExpExecArray | null; + while ((match = re.exec(code)) !== null) { + sloads.push({ + type, + line: lineNumber(code, match.index), + description, + recommendation, + }); + } + } + + if (sloads.length === 0) { + return { + detected: false, + sloads: [], + message: "No expensive SLOAD operations detected.", + suggestion: "", + }; + } + + return { + detected: true, + sloads, + message: `Detected ${sloads.length} expensive SLOAD pattern(s).`, + suggestion: + "Cache storage reads in local variables and avoid SLOAD operations inside loops to reduce gas costs.", + }; +} diff --git a/rules/security/signatures/detect-unsafe-signature-recovery.ts b/rules/security/signatures/detect-unsafe-signature-recovery.ts new file mode 100644 index 0000000..391f33b --- /dev/null +++ b/rules/security/signatures/detect-unsafe-signature-recovery.ts @@ -0,0 +1,96 @@ +export interface UnsafeSignatureRecovery { + type: string; + line: number; + description: string; + recommendation: string; +} + +export interface UnsafeSignatureRecoveryResult { + detected: boolean; + violations: UnsafeSignatureRecovery[]; + message: string; + suggestion: string; +} + +interface SignatureRecoveryPattern { + type: string; + pattern: RegExp; + description: string; + recommendation: string; +} + +const SIGNATURE_PATTERNS: SignatureRecoveryPattern[] = [ + { + type: "ecrecover-usage", + pattern: /\becrecover\s*\(/g, + description: + "ecrecover() without proper signature malleability protections may allow signature forgery.", + recommendation: + "Use OpenZeppelin ECDSA library which handles malleability (v/r/s validation, EIP-2).", + }, + { + type: "no-malleability-check", + pattern: /ecrecover\s*\([^)]*\)\s*(?![^;]*\brequire\b)/g, + description: + "ecrecover() result used without checking for signature malleability or invalid signature values.", + recommendation: + "Validate the s value is in the lower half of the secp256k1 curve and v is 27 or 28 (EIP-2).", + }, + { + type: "unchecked-recovery-result", + pattern: /address\s+\w+\s*=\s*ecrecover\s*\([^)]*\)\s*;/g, + description: + "ecrecover() returns address(0) for invalid signatures, but the result is not validated.", + recommendation: + "Always check that the recovered address is not address(0) and matches the expected signer.", + }, +]; + +function lineNumber(code: string, index: number): number { + let line = 1; + for (let i = 0; i < index; i++) { + if (code[i] === "\n") line++; + } + return line; +} + +export function detectUnsafeSignatureRecovery( + code: string, +): UnsafeSignatureRecoveryResult { + const violations: UnsafeSignatureRecovery[] = []; + + for (const { + type, + pattern, + description, + recommendation, + } of SIGNATURE_PATTERNS) { + const re = new RegExp(pattern.source, "g"); + let match: RegExpExecArray | null; + while ((match = re.exec(code)) !== null) { + violations.push({ + type, + line: lineNumber(code, match.index), + description, + recommendation, + }); + } + } + + if (violations.length === 0) { + return { + detected: false, + violations: [], + message: "No unsafe signature recovery patterns detected.", + suggestion: "", + }; + } + + return { + detected: true, + violations, + message: `Detected ${violations.length} unsafe signature recovery pattern(s).`, + suggestion: + "Use OpenZeppelin ECDSA library for safe signature recovery with malleability protections.", + }; +} diff --git a/src/analysis/stellar/storage-layout/index.ts b/src/analysis/stellar/storage-layout/index.ts new file mode 100644 index 0000000..3d16516 --- /dev/null +++ b/src/analysis/stellar/storage-layout/index.ts @@ -0,0 +1,6 @@ +export { SorobanStorageLayoutAnalyzer } from "./storage-layout-analyzer"; +export type { + StorageLayoutEntry, + StorageLayoutWarning, + StorageLayoutReport, +} from "./storage-layout-analyzer"; diff --git a/src/analysis/stellar/storage-layout/storage-layout-analyzer.spec.ts b/src/analysis/stellar/storage-layout/storage-layout-analyzer.spec.ts new file mode 100644 index 0000000..322c275 --- /dev/null +++ b/src/analysis/stellar/storage-layout/storage-layout-analyzer.spec.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "@jest/globals"; +import { SorobanStorageLayoutAnalyzer } from "./storage-layout-analyzer"; + +describe("SorobanStorageLayoutAnalyzer", () => { + it("builds storage layout from set operations", () => { + const source = ` +use soroban_sdk::{contract, contractimpl, Env, Symbol}; + +#[contract] +pub struct Counter; + +#[contractimpl] +impl Counter { + pub fn init(env: Env) { + env.storage().instance().set(&Symbol::new(&env, "count"), &0u32); + env.storage().instance().set(&Symbol::new(&env, "owner"), &env.current_contract_address()); + } +} +`; + + const analyzer = new SorobanStorageLayoutAnalyzer(source, "counter.rs"); + const report = analyzer.analyze(); + + expect(report.contractName).toBe("Counter"); + expect(report.entries.length).toBeGreaterThanOrEqual(2); + expect(report.summary).toContain("storage layout"); + }); + + it("warns on duplicate storage keys", () => { + const source = ` +use soroban_sdk::{contract, contractimpl, Env, Symbol}; + +#[contract] +pub struct Dup; + +#[contractimpl] +impl Dup { + pub fn set_a(env: Env) { + env.storage().instance().set(&Symbol::new(&env, "key"), &1u32); + env.storage().instance().set(&Symbol::new(&env, "key"), &2u32); + } +} +`; + + const analyzer = new SorobanStorageLayoutAnalyzer(source, "dup.rs"); + const report = analyzer.analyze(); + + expect(report.warnings.some((w) => w.severity === "high")).toBe(true); + }); + + it("returns warning when no storage entries found", () => { + const source = ` +use soroban_sdk::{contract, contractimpl, Env}; + +#[contract] +pub struct Empty; + +#[contractimpl] +impl Empty { + pub fn noop(env: Env) {} +} +`; + + const analyzer = new SorobanStorageLayoutAnalyzer(source, "empty.rs"); + const report = analyzer.analyze(); + + expect( + report.warnings.some((w) => w.message.includes("No storage entries")), + ).toBe(true); + }); +}); diff --git a/src/analysis/stellar/storage-layout/storage-layout-analyzer.ts b/src/analysis/stellar/storage-layout/storage-layout-analyzer.ts new file mode 100644 index 0000000..6179e7f --- /dev/null +++ b/src/analysis/stellar/storage-layout/storage-layout-analyzer.ts @@ -0,0 +1,166 @@ +export interface StorageLayoutEntry { + name: string; + type: string; + slot: number; + offset: number; + kind: 'instance' | 'persistent' | 'temporary'; + lineNumber: number; +} + +export interface StorageLayoutWarning { + message: string; + lineNumber: number; + severity: 'low' | 'medium' | 'high'; +} + +export interface StorageLayoutReport { + contractName: string; + entries: StorageLayoutEntry[]; + warnings: StorageLayoutWarning[]; + summary: string; +} + +interface RawStorageOp { + name: string; + kind: 'instance' | 'persistent' | 'temporary'; + lineNumber: number; +} + +export class SorobanStorageLayoutAnalyzer { + private source: string; + private filePath: string; + + constructor(source: string, filePath: string) { + this.source = source; + this.filePath = filePath; + } + + analyze(): StorageLayoutReport { + const contractName = this.extractContractName(); + const rawOps = this.collectStorageOps(); + const entries = this.deduplicateEntries(rawOps); + const warnings = this.analyzeLayoutWarnings(rawOps, entries); + + return { + contractName, + entries, + warnings, + summary: this.buildSummary(contractName, entries, warnings), + }; + } + + private extractContractName(): string { + const match = this.source.match(/(?:pub\s+)?struct\s+(\w+)/); + return match ? match[1] : 'UnknownContract'; + } + + private collectStorageOps(): RawStorageOp[] { + const ops: RawStorageOp[] = []; + const setPattern = /(?:env\.)?storage\(\)\.(\w+)\(\)\.set\s*\(\s*(?:&)?(?:Symbol::new\s*\([^,]+,\s*"([^"]+)"\)|([A-Za-z_]\w*))\s*,/g; + let match: RegExpExecArray | null; + + while ((match = setPattern.exec(this.source)) !== null) { + const kind = match[1] as 'instance' | 'persistent' | 'temporary'; + const name = match[2] || match[3] || 'unknown'; + ops.push({ name, kind, lineNumber: this.getLineNumber(match.index) }); + } + + return ops; + } + + private deduplicateEntries(ops: RawStorageOp[]): StorageLayoutEntry[] { + const seen = new Set(); + return ops.filter(op => { + const key = `${op.kind}:${op.name}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }).map((op, idx) => ({ + name: op.name, + type: this.inferType(op.name), + slot: idx, + offset: 0, + kind: op.kind, + lineNumber: op.lineNumber, + })); + } + + private inferType(keyName: string): string { + const typeHints: Record = { + count: 'u32', + balance: 'i128', + owner: 'Address', + admin: 'Address', + total_supply: 'i128', + name: 'String', + symbol: 'Symbol', + paused: 'bool', + }; + return typeHints[keyName] || 'unknown'; + } + + private analyzeLayoutWarnings(rawOps: RawStorageOp[], entries: StorageLayoutEntry[]): StorageLayoutWarning[] { + const warnings: StorageLayoutWarning[] = []; + const seen = new Map(); + + for (const op of rawOps) { + const key = `${op.kind}:${op.name}`; + if (seen.has(key)) { + warnings.push({ + message: `Duplicate storage key '${op.name}' found in '${op.kind}' storage.`, + lineNumber: op.lineNumber, + severity: 'high', + }); + } + seen.set(key, (seen.get(key) || 0) + 1); + } + + for (const entry of entries) { + if (entry.type === 'unknown') { + warnings.push({ + message: `Storage key '${entry.name}' has unknown type. Consider adding explicit type annotations.`, + lineNumber: entry.lineNumber, + severity: 'low', + }); + } + + if (entry.kind === 'persistent' && entry.name.startsWith('tmp_')) { + warnings.push({ + message: `Persistent storage key '${entry.name}' uses 'tmp_' prefix. Consider using temporary storage instead.`, + lineNumber: entry.lineNumber, + severity: 'medium', + }); + } + } + + if (entries.length === 0) { + warnings.push({ + message: 'No storage entries detected. Contract may not use persistent state.', + lineNumber: 1, + severity: 'low', + }); + } + + return warnings; + } + + private buildSummary( + contractName: string, + entries: StorageLayoutEntry[], + warnings: StorageLayoutWarning[], + ): string { + const instanceCount = entries.filter(e => e.kind === 'instance').length; + const persistentCount = entries.filter(e => e.kind === 'persistent').length; + const temporaryCount = entries.filter(e => e.kind === 'temporary').length; + + return ( + `${contractName} storage layout: ${entries.length} key(s) ` + + `(${instanceCount} instance, ${persistentCount} persistent, ${temporaryCount} temporary) ` + + `with ${warnings.length} warning(s).` + ); + } + + private getLineNumber(offset: number): number { + return (this.source.slice(0, offset).match(/\n/g) ?? []).length + 1; + } +} diff --git a/tests/rules/detect-expensive-memory-copies.spec.ts b/tests/rules/detect-expensive-memory-copies.spec.ts new file mode 100644 index 0000000..6c92d24 --- /dev/null +++ b/tests/rules/detect-expensive-memory-copies.spec.ts @@ -0,0 +1,30 @@ +import { detectExpensiveMemoryCopies } from '../../rules/optimization/memory/detect-expensive-memory-copies'; + +describe('detectExpensiveMemoryCopies', () => { + it('flags unnecessary .clone() calls', () => { + const code = `let copy = original.clone();`; + const result = detectExpensiveMemoryCopies(code); + expect(result.detected).toBe(true); + expect(result.copies.some(c => c.type === 'unnecessary-clone')).toBe(true); + }); + + it('flags String::from conversions', () => { + const code = `let s = String::from(name);`; + const result = detectExpensiveMemoryCopies(code); + expect(result.detected).toBe(true); + expect(result.copies.some(c => c.type === 'string-to-string-copy')).toBe(true); + }); + + it('flags vec copy operations', () => { + const code = `let all = items.to_vec();`; + const result = detectExpensiveMemoryCopies(code); + expect(result.detected).toBe(true); + expect(result.copies.some(c => c.type === 'vec-copy')).toBe(true); + }); + + it('returns clean for reference usage', () => { + const code = `fn process(data: &[u8]) -> u32 { data.len() as u32 }`; + const result = detectExpensiveMemoryCopies(code); + expect(result.detected).toBe(false); + }); +}); diff --git a/tests/rules/detect-expensive-sload-operations.spec.ts b/tests/rules/detect-expensive-sload-operations.spec.ts new file mode 100644 index 0000000..b6a9aad --- /dev/null +++ b/tests/rules/detect-expensive-sload-operations.spec.ts @@ -0,0 +1,27 @@ +import { detectExpensiveSLOADOperations } from '../../rules/optimization/storage/detect-expensive-sload-operations'; + +describe('detectExpensiveSLOADOperations', () => { + it('flags repeated SLOAD operations', () => { + const code = `let a = storage().instance().get(&key); +let b = storage().instance().get(&key);`; + const result = detectExpensiveSLOADOperations(code); + expect(result.detected).toBe(true); + expect(result.sloads.some(s => s.type === 'repeated-sload')).toBe(true); + }); + + it('flags SLOAD in loop', () => { + const code = `for i in 0..n { + let val = storage().instance().get(&key); +}`; + const result = detectExpensiveSLOADOperations(code); + expect(result.detected).toBe(true); + }); + + it('returns clean for single cached read', () => { + const code = `let val = storage().instance().get(&key); +process(val); +update(val);`; + const result = detectExpensiveSLOADOperations(code); + expect(result.detected).toBe(false); + }); +}); diff --git a/tests/rules/detect-unsafe-signature-recovery.spec.ts b/tests/rules/detect-unsafe-signature-recovery.spec.ts new file mode 100644 index 0000000..64ee641 --- /dev/null +++ b/tests/rules/detect-unsafe-signature-recovery.spec.ts @@ -0,0 +1,22 @@ +import { detectUnsafeSignatureRecovery } from '../../rules/security/signatures/detect-unsafe-signature-recovery'; + +describe('detectUnsafeSignatureRecovery', () => { + it('flags raw ecrecover usage', () => { + const code = `address signer = ecrecover(hash, v, r, s);`; + const result = detectUnsafeSignatureRecovery(code); + expect(result.detected).toBe(true); + expect(result.violations.some(v => v.type === 'ecrecover-usage')).toBe(true); + }); + + it('flags unchecked ecrecover result', () => { + const code = `address recovered = ecrecover(hash, v, r, s);`; + const result = detectUnsafeSignatureRecovery(code); + expect(result.detected).toBe(true); + }); + + it('returns clean for proper ECDSA usage', () => { + const code = `address signer = ECDSA.recover(hash, signature);`; + const result = detectUnsafeSignatureRecovery(code); + expect(result.detected).toBe(false); + }); +});