diff --git a/packages/analyzers/soroban/deployment/__tests__/deployment-config-analyzer.spec.ts b/packages/analyzers/soroban/deployment/__tests__/deployment-config-analyzer.spec.ts new file mode 100644 index 00000000..440df6e2 --- /dev/null +++ b/packages/analyzers/soroban/deployment/__tests__/deployment-config-analyzer.spec.ts @@ -0,0 +1,103 @@ +import { + analyzeDeploymentConfig, + detectDeploymentConfigFindings, +} from '../deployment-config-analyzer'; + +const VALID_MAINNET = ` +network = "mainnet" +rpc_url = "https://rpc.stellar.org" +owner = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +wasm_hash = "b9f0b8cbe0b79bca" +fee = 100 +`; + +const MISSING_REQUIRED = ` +[deploy] +rpc_url = "https://rpc.stellar.org" +owner = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +`; + +const INVALID_SETTINGS = ` +network = "not-a-real-network" +rpc_url = "localhost:8000" +owner = "bob" +wasm_hash = "0x1234" +fee_bump = 500 +max_fee = "lots" +`; + +const TESTNET_FRAGMENT = ` +network = "testnet" +wasm = "target/wasm32-unknown-unknown/release/contract.wasm" +`; + +describe('SorobanDeploymentConfigAnalyzer (#927)', () => { + it('parses settings from a flat deployment configuration', () => { + const report = analyzeDeploymentConfig(VALID_MAINNET); + const keys = report.parsedSettings.map((s) => s.key); + expect(keys).toEqual( + expect.arrayContaining(['network', 'rpc_url', 'owner', 'wasm_hash', 'fee']), + ); + expect(report.network).toBe('mainnet'); + expect(report.rpcUrl).toBe('https://rpc.stellar.org'); + expect(report.owner).toBe('GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'); + }); + + it('passes a complete, valid production configuration', () => { + const report = analyzeDeploymentConfig(VALID_MAINNET); + expect(report.valid).toBe(true); + expect(report.findings).toHaveLength(0); + expect(report.missingRequired).toHaveLength(0); + }); + + it('detects missing network, rpc_url and owner', () => { + const report = analyzeDeploymentConfig(MISSING_REQUIRED); + expect(report.valid).toBe(false); + expect(report.network).toBeUndefined(); + // network is missing -> critical; owner missing here but no production target. + const missingKey = report.findings + .filter((f) => f.title.startsWith('Missing deployment configuration:')) + .map((f) => f.key); + expect(missingKey).toEqual(expect.arrayContaining(['network', 'owner'])); + const networkFinding = report.findings.find((f) => f.key === 'network'); + expect(networkFinding?.severity).toBe('critical'); + }); + + it('flags unsupported keys and invalid values', () => { + const report = analyzeDeploymentConfig(INVALID_SETTINGS); + const unsupported = report.findings.find((f) => f.key === 'fee_bump'); + expect(unsupported).toBeDefined(); + expect(unsupported?.severity).toBe('low'); + expect(unsupported?.title).toBe('Unsupported deployment setting'); + + const badNetwork = report.findings.find((f) => f.key === 'network'); + expect(badNetwork?.severity).toBe('high'); + + const badRpc = report.findings.find((f) => f.key === 'rpc_url'); + expect(badRpc?.severity).toBe('high'); + + const badFee = report.findings.find((f) => f.key === 'max_fee'); + expect(badFee?.severity).toBe('medium'); + + const badOwner = report.findings.find((f) => f.key === 'owner'); + expect(badOwner?.severity).toBe('medium'); + expect(report.valid).toBe(false); + }); + + it('accepts non-production fragments without a production owner requirement', () => { + const report = analyzeDeploymentConfig(TESTNET_FRAGMENT); + const hasConfigFindings = report.findings.filter((f) => f.key === 'owner'); + // Owner missing on testnet is only 'low', not blocking. + const ownerFinding = hasConfigFindings.find((f) => f.title.includes('owner')); + expect(ownerFinding).toBeDefined(); + expect(['low', 'medium']).toContain(ownerFinding?.severity); + // An artifact (wasm) is declared, so no "no deploy artifact" info finding. + expect(report.findings.some((f) => f.title === 'No deploy artifact specified')).toBe(false); + }); + + it('exposes the findings via the convenience wrapper', () => { + const findings = detectDeploymentConfigFindings(MISSING_REQUIRED); + expect(findings.length).toBeGreaterThan(0); + expect(findings[0].ruleId).toBe('soroban-deployment-config'); + }); +}); \ No newline at end of file diff --git a/packages/analyzers/soroban/deployment/deployment-config-analyzer.ts b/packages/analyzers/soroban/deployment/deployment-config-analyzer.ts new file mode 100644 index 00000000..5df37c47 --- /dev/null +++ b/packages/analyzers/soroban/deployment/deployment-config-analyzer.ts @@ -0,0 +1,370 @@ +/** + * Issue #927 — Soroban Deployment Configuration Analyzer + * + * Parses Soroban deployment configuration (invoke.soroban / project.toml / + * deploy config), validates that the settings it declares are supported and + * well-formed, and reports both unknown/invalid settings and configuration + * that is required for a correct deployment but missing. + * + * Incorrect deployment settings can produce inefficient or unreliable + * contract releases — a contract deployed against the wrong network, without + * an RPC endpoint, or without an owner that any future upgrade checks — so + * this analyzer flags them at configuration time rather than at deploy time. + */ + +export type DeploymentSeverity = 'critical' | 'high' | 'medium' | 'low' | 'info'; + +export interface DeploymentConfigSetting { + key: string; + value: string; + section: string; + line: number; +} + +export interface DeploymentConfigFinding { + ruleId: string; + severity: DeploymentSeverity; + title: string; + /** Config source the finding relates to (e.g. `invoke.soroban`). */ + source: string; + key?: string; + message: string; + suggestion: string; + line?: number; +} + +export interface DeploymentConfigReport { + findings: DeploymentConfigFinding[]; + /** Parsed key/value settings, in file order. */ + parsedSettings: DeploymentConfigSetting[]; + network?: string; + rpcUrl?: string; + owner?: string; + /** Required keys that were not present in the configuration. */ + missingRequired: string[]; + /** True when no critical or high-severity finding was raised. */ + valid: boolean; +} + +/** Networks a Soroban deployment may legitimately target. */ +const KNOWN_NETWORKS = new Set([ + 'mainnet', + 'public', + 'pubnet', + 'testnet', + 'futurenet', + 'standalone', + 'local', + 'development', +]); + +const PRODUCTION_NETWORKS = new Set(['mainnet', 'public', 'pubnet']); + +/** Deployment settings this analyzer understands, per canonical key. */ +const SUPPORTED_KEYS = new Set([ + 'network', + 'rpc_url', + 'rpc', + 'horizon_url', + 'owner', + 'admin', + 'account', + 'source', + 'deployer', + 'secret_key', + 'wasm', + 'wasm_hash', + 'contract', + 'contract_id', + 'salt', + 'fee', + 'max_fee', + 'fee_rate', + 'fee_percentage', + 'timeout', +]); + +/** Keys that must resolve to a number. */ +const NUMERIC_KEYS = new Set(['fee', 'max_fee', 'fee_rate', 'fee_percentage', 'timeout']); + +/** Keys that must resolve to an HTTP(S) URL. */ +const URL_KEYS = new Set(['rpc_url', 'rpc', 'horizon_url']); + +const STELLAR_PUBKEY = /^[A-Z2-7]{56}$/; + +interface TomlBucket { + lines: number[]; + values: Map; +} + +/** Extremely tolerant line-oriented TOML bucketter for deployment config. */ +function bucketSections(content: string): { + sections: Map; + root: TomlBucket; +} { + const sections = new Map(); + const root: TomlBucket = { lines: [], values: new Map() }; + let current: string | null = null; + + const ensure = (name: string): TomlBucket => { + if (!sections.has(name)) { + sections.set(name, { lines: [], values: new Map() }); + } + return sections.get(name)!; + }; + + content.split(/\r?\n/).forEach((raw, idx) => { + const line = raw.trim(); + if (line.startsWith('#')) return; + const header = /^\[([^\]]+)\]$/.exec(line); + if (header) { + current = header[1]; + ensure(current); + return; + } + const kv = /^([a-zA-Z0-9_-]+)\s*=\s*(.+)$/.exec(line); + if (!kv) return; + const entry = { value: kv[2].trim(), line: idx + 1 }; + if (current) { + const sec = ensure(current); + sec.lines.push(idx + 1); + sec.values.set(kv[1], entry); + } else { + root.lines.push(idx + 1); + root.values.set(kv[1], entry); + } + }); + + return { sections, root }; +} + +function valueOf(values: Map): Map { + const out = new Map(); + for (const [k, v] of values) { + out.set(k, stripQuotes(v.value)); + } + return out; +} + +function stripQuotes(raw: string): string { + const s = raw.trim(); + const m = s.match(/^"((?:[^"\\]|\\.)*)"$/); + if (m) return m[1]; + return s; +} + +/** + * Parse deployment configuration and validate its settings. + * + * @param configuration Deployment config text (invoke.soroban, project.toml, + * or a deploy config fragment). May contain `[section]` blocks or a flat set + * of root-level keys. + * @param source A human-readable label for the config source, used in findings. + */ +export function analyzeDeploymentConfig( + configuration: string, + source = 'invoke.soroban', +): DeploymentConfigReport { + const { sections, root } = bucketSections(configuration); + const settings: DeploymentConfigSetting[] = []; + const findings: DeploymentConfigFinding[] = []; + const missing: string[] = []; + + // Merge root keys with the most common deployment sections, preferring + // section-scoped values over root duplicates. + const merged = new Map(); + const mergeInto = (values: Map, section: string) => { + const normalized = valueOf(values); + for (const [key, value] of normalized) { + merged.set(key, { value, section, line: values.get(key)!.line }); + settings.push({ key, value, section, line: values.get(key)!.line }); + } + }; + + mergeInto(root.values, '__root'); + for (const [name, bucket] of sections) { + mergeInto(bucket.values, name); + } + + const warnUnknown = (key: string, value: string, line?: number) => { + findings.push({ + ruleId: 'soroban-deployment-config', + severity: 'low', + title: 'Unsupported deployment setting', + source, + key, + line, + message: + `Deployment configuration contains '${key}' = '${value}', which is not a ` + + `recognized deployment setting. It may be a typo or an unsupported option that ` + + `will be silently ignored.`, + suggestion: + `Remove '${key}' or rename it to a supported key (${[...SUPPORTED_KEYS].join(', ')}).`, + }); + }; + + const pushMissing = (key: string, severity: DeploymentSeverity, why: string, suggestion: string) => { + missing.push(key); + findings.push({ + ruleId: 'soroban-deployment-config', + severity, + title: `Missing deployment configuration: ${key}`, + source, + key, + message: `Deployment configuration does not declare '${key}'. ${why}`, + suggestion, + }); + }; + + const pushInvalid = ( + key: string, + value: string, + severity: DeploymentSeverity, + why: string, + line?: number, + ) => { + findings.push({ + ruleId: 'soroban-deployment-config', + severity, + title: `Invalid deployment setting: ${key}`, + source, + key, + line, + message: `Deployment setting '${key}' = '${value}' is invalid. ${why}`, + suggestion: `Set '${key}' to a supported, well-formed value before deploying.`, + }); + }; + + // Feed the merged settings through the supported/validation gates. + for (const [key, { value, line }] of merged) { + if (!SUPPORTED_KEYS.has(key)) { + warnUnknown(key, value, line); + continue; + } + if (NUMERIC_KEYS.has(key)) { + const n = Number(value); + if (!Number.isFinite(n) || n < 0) { + pushInvalid(key, value, 'medium', `expected a non-negative number, got '${value}'.`, line); + } + continue; + } + if (URL_KEYS.has(key)) { + if (!/^https?:\/\//i.test(value)) { + pushInvalid(key, value, 'high', `expected an http(s) URL, got '${value}'.`, line); + } + } + } + + // Resolve the canonical read-mostly values. + const pick = (keys: string[]): string | undefined => { + for (const k of keys) { + if (merged.has(k)) return merged.get(k)!.value; + } + return undefined; + }; + const pickEntry = (keys: string[]): { key: string; value: string; line: number } | undefined => { + for (const k of keys) { + if (merged.has(k)) return { key: k, ...merged.get(k)! }; + } + return undefined; + }; + const network = stripQuotes(pick(['network']) ?? ''); + const rpcUrl = pick(['rpc_url', 'rpc']); + const ownerEntry = pickEntry(['owner', 'admin', 'account', 'source', 'deployer']); + const owner = ownerEntry?.value; + const wasmHash = pick(['wasm_hash', 'wasm', 'contract', 'contract_id']); + + const productionTarget = network ? PRODUCTION_NETWORKS.has(network.toLowerCase()) : false; + + // Missing-required detection. + if (!network) { + pushMissing( + 'network', + 'critical', + 'Without a target network the deployment may end up on the wrong network.', + `Set 'network' to one of: ${[...KNOWN_NETWORKS].join(', ')}.`, + ); + } else if (!KNOWN_NETWORKS.has(network.toLowerCase())) { + pushInvalid( + 'network', + network, + 'high', + `expected one of ${[...KNOWN_NETWORKS].join(', ')}, got '${network}'.`, + merged.get('network')!.line, + ); + } + + if (!rpcUrl) { + pushMissing( + 'rpc_url', + productionTarget ? 'critical' : 'medium', + productionTarget + ? 'Production deployments without a pinned RPC endpoint cannot be verified or reproduced reliably.' + : 'A fallback/default RPC endpoint may be used, which is not reproducible.', + `Set 'rpc_url' to the network's RPC endpoint, e.g. https://rpc.stellar.org.`, + ); + } + + if (!owner) { + pushMissing( + 'owner', + productionTarget ? 'high' : 'low', + productionTarget + ? 'No deploy owner/source account is declared, so post-deployment upgrades are unanchored.' + : 'No deploy owner/source account is declared.', + `Set 'owner' (or 'admin'/'account'/'source') to the deploying Stellar public key.`, + ); + } else { + const m = owner.trim().match(/^[GACST][0-9A-Z]{55}$/); + if (!m && ownerEntry) { + findings.push({ + ruleId: 'soroban-deployment-config', + severity: 'medium', + title: 'Owner does not look like a Stellar public key', + source, + key: ownerEntry.key, + line: ownerEntry.line, + message: + `Deployment owner '${owner}' is not a 56-character Stellar public key (G/C/S...).`, + suggestion: + 'Use the deploying account public key (starts with G for the account address, or C for a contract address).', + }); + } + } + + if (!wasmHash) { + findings.push({ + ruleId: 'soroban-deployment-config', + severity: 'info', + title: 'No deploy artifact specified', + source, + key: 'wasm', + message: + 'No build artifact (wasm/wasm_hash/contract_id) is declared, so this configuration cannot pin which contract is deployed.', + suggestion: + "Declare 'wasm' (path to the built .wasm, e.g. target/wasm32-unknown-unknown/release/contract.wasm) or 'wasm_hash' for a reproducible, pinned deployment.", + }); + } + + const criticalFindings = findings.filter((f) => f.severity === 'critical' || f.severity === 'high'); + + return { + findings: findings.sort((a, b) => (a.line ?? Infinity) - (b.line ?? Infinity)), + parsedSettings: settings, + network: network || undefined, + rpcUrl, + owner, + missingRequired: missing, + valid: criticalFindings.length === 0, + }; +} + +/** + * Convenience wrapper that reports only the findings. + */ +export function detectDeploymentConfigFindings( + configuration: string, + source?: string, +): DeploymentConfigFinding[] { + return analyzeDeploymentConfig(configuration, source).findings; +} \ No newline at end of file diff --git a/packages/analyzers/soroban/security/__tests__/unprotected-upgrade-analyzer.spec.ts b/packages/analyzers/soroban/security/__tests__/unprotected-upgrade-analyzer.spec.ts new file mode 100644 index 00000000..5794b154 --- /dev/null +++ b/packages/analyzers/soroban/security/__tests__/unprotected-upgrade-analyzer.spec.ts @@ -0,0 +1,72 @@ +import { + analyzeUnprotectedUpgrades, + UnprotectedUpgradeAnalyzer, +} from '../unprotected-upgrade-analyzer'; + +const UNPROTECTED_WASM_UPGRADE = ` +#[contractimpl] +impl UpgradeableImpl { + pub fn upgrade(env: Env, new_wasm: Bytes) { + env.update_current_contract_wasm(&new_wasm); + } + + pub fn set_implementation(env: Env, impl: Bytes) { + env.storage().instance().set(&Symbol::new(&env, "impl"), &impl); + } + + pub fn name(env: Env) -> Symbol { + Symbol::new(&env, "upgradeable") + } +} +`; + +const PROTECTED_UPGRADE = ` +#[contractimpl] +impl UpgradeableImpl { + pub fn upgrade(env: Env, admin: Address, new_wasm: Bytes) { + admin.require_auth(); + env.update_current_contract_wasm(&new_wasm); + } + + pub fn only_admin_upgrade(env: Env, new_wasm: Bytes) { + only_admin(); + env.update_current_contract_wasm(&new_wasm); + } +} +`; + +describe('SorobanUnprotectedUpgradeAnalyzer (#925)', () => { + it('flags upgrade functions invoked without access control as critical', () => { + const findings = analyzeUnprotectedUpgrades(UNPROTECTED_WASM_UPGRADE); + const upgrade = findings.find((f) => f.functionName === 'upgrade'); + expect(upgrade).toBeDefined(); + expect(upgrade?.severity).toBe('critical'); + expect(upgrade?.ruleId).toBe('soroban-unprotected-upgrade'); + expect(upgrade?.location.functionName).toBe('upgrade'); + expect(upgrade?.message).toContain('no access-control check'); + }); + + it('flags implementation-swap storage writes as upgrade functions', () => { + const findings = analyzeUnprotectedUpgrades(UNPROTECTED_WASM_UPGRADE); + const setImpl = findings.find((f) => f.functionName === 'set_implementation'); + expect(setImpl).toBeDefined(); + // Storage-backed implementation swap is still an uncontrolled upgrade path. + expect(setImpl?.severity).toBe('critical'); + }); + + it('does not treat read-only functions as upgrade functions', () => { + const findings = analyzeUnprotectedUpgrades(UNPROTECTED_WASM_UPGRADE); + expect(findings.some((f) => f.functionName === 'name')).toBe(false); + }); + + it('does not flag upgrade functions that authorize first', () => { + const findings = analyzeUnprotectedUpgrades(PROTECTED_UPGRADE); + expect(findings).toHaveLength(0); + }); + + it('exposes the analyzer class with a stable rule id', () => { + expect(UnprotectedUpgradeAnalyzer.RULE_ID).toBe('soroban-unprotected-upgrade'); + const findings = new UnprotectedUpgradeAnalyzer().analyze(UNPROTECTED_WASM_UPGRADE); + expect(findings.length).toBeGreaterThan(0); + }); +}); \ No newline at end of file diff --git a/packages/analyzers/soroban/security/index.ts b/packages/analyzers/soroban/security/index.ts index 64ba6cfd..bcf70859 100644 --- a/packages/analyzers/soroban/security/index.ts +++ b/packages/analyzers/soroban/security/index.ts @@ -1,2 +1,3 @@ export * from './missing-authorization-analyzer'; +export * from './unprotected-upgrade-analyzer'; export * from './unchecked-auth-parameter-analyzer'; diff --git a/packages/analyzers/soroban/security/unprotected-upgrade-analyzer.ts b/packages/analyzers/soroban/security/unprotected-upgrade-analyzer.ts new file mode 100644 index 00000000..52d56144 --- /dev/null +++ b/packages/analyzers/soroban/security/unprotected-upgrade-analyzer.ts @@ -0,0 +1,125 @@ +/** + * Issue #925 — Detect Unprotected Soroban Upgrade Functions (security analyzer) + * + * Identifies Soroban functions that can replace or migrate contract behaviour + * and are not protected by an access-control check. Unauthorized upgrades can + * swap the deployed implementation for a malicious one, so missing access + * control here is a security (not merely a governance) issue. + * + * This analyzer is a focused security pass over the same upgrade triggers the + * upgradeability analyzer tracks, but it reports per-function access-control + * gaps with a security severity rather than a mechanism inventory. + */ + +import { maskNonCode } from '../common/source-utils'; + +export type UpgradeSecuritySeverity = 'critical' | 'high' | 'medium'; + +export interface UnprotectedUpgradeFinding { + line: number; + ruleId: string; + severity: UpgradeSecuritySeverity; + functionName: string; + /** The upgrade trigger/mechanism found in the function. */ + trigger: string; + hasAuthorization: boolean; + authorizedBy?: string; + message: string; + suggestion: string; + location: { line: number; functionName: string }; +} + +/** Calls that can replace or migrate the deployed behaviour. */ +const UPGRADE_TRIGGER = + /update_current_contract_wasm\s*\(|\.set_code\s*\(|\.set_wasm\s*\(|set_wasm_hash\s*\(|set_implementation\s*\(|set_impl\s*\(|version_switch\s*\(|env\s*\.\s*deployer\s*\(|\.deploy_contract\s*\(/; + +/** Function names that conventionally perform upgrades. */ +const UPGRADE_FN_NAME = + /\b(upgrade|update_wasm|set_wasm|set_code|set_implementation|migrate|release_contract|update_contract|redeploy|replace_contract)\b/i; + +/** Access-control primitives that would protect an upgrade function. */ +const ACCESS_CONTROL = + /require_auth\s*\(|require_auth_for_args\s*\(|\.authenticate\s*\(|\b(admin|owner|admin_owner|governor|controller)\s*\.\s*require_auth\s*\(|\bonly_admin\s*\(|assert_admin\s*\(|check_admin\s*\(|ensure_admin\s*\(/; + +/** Direct wasm replacement is treated as the most severe uncontrolled path. */ +const DIRECT_REPLACEMENT = + /update_current_contract_wasm\s*\(|\.set_code\s*\(|\.set_wasm\s*\(|set_wasm_hash\s*\(/; + +function findUpgradeFunctions(source: string): Array<{ name: string; body: string; line: number }> { + const masked = maskNonCode(source); + const blocks: Array<{ name: string; body: string; line: number }> = []; + const re = /\bfn\s+([A-Za-z_][A-Za-z0-9_]*)\s*\(/g; + let m: RegExpExecArray | null; + + while ((m = re.exec(masked)) !== null) { + // Rough body estimate: braces from the first '{' after the signature. + const braceIdx = masked.indexOf('{', m.index); + if (braceIdx === -1) continue; + let depth = 0; + let end = -1; + for (let i = braceIdx; i < masked.length; i++) { + if (masked[i] === '{') depth++; + else if (masked[i] === '}') { + depth--; + if (depth === 0) { + end = i; + break; + } + } + } + if (end === -1) continue; + const line = masked.slice(0, m.index).split('\n').length; + blocks.push({ + name: m[1], + body: masked.slice(braceIdx, end + 1), + line, + }); + } + return blocks; +} + +/** + * Detect upgrade functions that lack an access-control check. + */ +export function analyzeUnprotectedUpgrades(sourceCode: string): UnprotectedUpgradeFinding[] { + const findings: UnprotectedUpgradeFinding[] = []; + + for (const fn of findUpgradeFunctions(sourceCode)) { + const isUpgradeName = UPGRADE_FN_NAME.test(fn.name); + const hasTrigger = UPGRADE_TRIGGER.test(fn.body); + if (!isUpgradeName && !hasTrigger) continue; + + const hasAuth = ACCESS_CONTROL.test(fn.body); + if (hasAuth) continue; + + const direct = DIRECT_REPLACEMENT.test(fn.body) || isUpgradeName; + const trigger = hasTrigger ? 'upgrade trigger in body' : `fn name '${fn.name}'`; + + findings.push({ + line: fn.line, + ruleId: 'soroban-unprotected-upgrade', + severity: direct ? 'critical' : 'high', + functionName: fn.name, + trigger, + hasAuthorization: false, + message: + `Upgrade function '${fn.name}' (${trigger}) has no access-control check, so an ` + + `unauthorized caller could replace or migrate the deployed contract behaviour.`, + suggestion: + `Add an explicit authorization check at the start of '${fn.name}': ` + + `\`{admin}.require_auth()\` for the privileged Address, or \`require_auth_for_args\`. ` + + `Prefer a timelock or multi-sig admin for upgradeable contracts, and emit an upgrade event.`, + location: { line: fn.line, functionName: fn.name }, + }); + } + + return findings.sort((a, b) => a.line - b.line); +} + +export class UnprotectedUpgradeAnalyzer { + public static readonly RULE_ID = 'soroban-unprotected-upgrade'; + + analyze(sourceCode: string): UnprotectedUpgradeFinding[] { + return analyzeUnprotectedUpgrades(sourceCode); + } +} \ No newline at end of file diff --git a/packages/analyzers/soroban/storage/__tests__/mutable-upgrade-config-analyzer.spec.ts b/packages/analyzers/soroban/storage/__tests__/mutable-upgrade-config-analyzer.spec.ts new file mode 100644 index 00000000..d28f0c79 --- /dev/null +++ b/packages/analyzers/soroban/storage/__tests__/mutable-upgrade-config-analyzer.spec.ts @@ -0,0 +1,81 @@ +import { + analyzeMutableUpgradeConfig, + detectMutableUpgradeConfigWrites, + MutableUpgradeConfigAnalyzer, +} from '../mutable-upgrade-config-analyzer'; + +const MUTABLE_UPGRADE_CONFIG = ` +#[contractimpl] +impl UpgradeableImpl { + pub fn set_wasm_hash(env: Env, wasm_hash: BytesN<32>) { + env.storage().persistent().set(&Symbol::new(&env, "wasm_hash"), &wasm_hash); + } + + pub fn set_impl(env: Env, admin: Address, impl: Address) { + admin.require_auth(); + env.storage().instance().set(&Symbol::new(&env, "implementation"), &impl); + } + + pub fn set_owner(env: Env, owner: Address) { + env.storage().instance().set(&Symbol::new(&env, "owner"), &owner); + } +} +`; + +describe('MutableUpgradeConfigAnalyzer (#926)', () => { + it('tracks writes to upgrade-configuration storage keys', () => { + const writes = detectMutableUpgradeConfigWrites(MUTABLE_UPGRADE_CONFIG); + const keys = writes.map((w) => w.key); + expect(keys).toContainEqual(expect.stringContaining('wasm_hash')); + expect(keys).toContainEqual(expect.stringContaining('implementation')); + // 'owner' is not an upgrade-configuration key by this heuristic. + expect(keys.some((k) => k.includes('owner'))).toBe(false); + }); + + it('reports unauthenticated writes to upgrade configuration as findings', () => { + const report = analyzeMutableUpgradeConfig(MUTABLE_UPGRADE_CONFIG); + expect(report.totalWriteCount).toBe(2); + expect(report.unsafeWriteCount).toBe(1); + + const unsafe = report.findings.find((f) => f.functionName === 'set_wasm_hash'); + expect(unsafe).toBeDefined(); + expect(unsafe?.severity).toBe('high'); + expect(unsafe?.ruleId).toBe('soroban-mutable-upgrade-config'); + expect(unsafe?.message).toContain('without an authorization check'); + }); + + it('does not flag authorized writes to upgrade configuration', () => { + const report = analyzeMutableUpgradeConfig(MUTABLE_UPGRADE_CONFIG); + const writes = report.writes.filter((w) => w.fn === 'set_impl'); + expect(writes).toHaveLength(1); + expect(writes[0].hasAuthorization).toBe(true); + expect(report.findings.some((f) => f.functionName === 'set_impl')).toBe(false); + }); + + it('returns recommendations when unsafe mutation paths exist', () => { + const report = analyzeMutableUpgradeConfig(MUTABLE_UPGRADE_CONFIG); + expect(report.recommendations.some((r) => r.includes('unauthenticated'))).toBe(true); + }); + + it('reports clean contracts without findings', () => { + const clean = ` + #[contractimpl] + impl CounterImpl { + pub fn increment(env: Env, key: Symbol) -> u32 { + let n: u32 = env.storage().instance().get(&key).unwrap_or(0); + env.storage().instance().set(&key, &(n + 1)); + n + 1 + } + } + `; + const report = analyzeMutableUpgradeConfig(clean); + expect(report.totalWriteCount).toBe(0); + expect(report.findings).toHaveLength(0); + }); + + it('exposes the analyzer class with a stable rule id', () => { + expect(MutableUpgradeConfigAnalyzer.RULE_ID).toBe('soroban-mutable-upgrade-config'); + const report = new MutableUpgradeConfigAnalyzer().analyze(MUTABLE_UPGRADE_CONFIG); + expect(report.unsafeWriteCount).toBe(1); + }); +}); \ No newline at end of file diff --git a/packages/analyzers/soroban/storage/index.ts b/packages/analyzers/soroban/storage/index.ts index 9b344eac..bf0ae950 100644 --- a/packages/analyzers/soroban/storage/index.ts +++ b/packages/analyzers/soroban/storage/index.ts @@ -6,3 +6,4 @@ export * from "./redundant-write-analyzer"; export * from "./storage-entry-classifier"; export * from "./inefficient-temporary-storage-analyzer"; export * from "./storage-footprint-expansion-analyzer"; +export * from "./mutable-upgrade-config-analyzer"; diff --git a/packages/analyzers/soroban/storage/mutable-upgrade-config-analyzer.ts b/packages/analyzers/soroban/storage/mutable-upgrade-config-analyzer.ts new file mode 100644 index 00000000..3474565c --- /dev/null +++ b/packages/analyzers/soroban/storage/mutable-upgrade-config-analyzer.ts @@ -0,0 +1,187 @@ +/** + * Issue #926 — Detect Mutable Soroban Upgrade Configuration (storage analyzer) + * + * Identifies upgrade configuration stored in contract ledger state — the keys + * that record what the contract may be upgraded to (wasm hashes, implementation + * addresses, upgrade targets) — and tracks every write to those keys, reporting + * mutation paths that are not protected by an authorization check. + * + * Attackers who can mutate upgrade configuration without authentication can + * redirect a future upgrade to a malicious implementation, so unsafe mutation + * paths are reported as security findings, not just housekeeping. + */ + +import { + maskNonCode, + createLineResolver, + extractFunctions, + extractArgs, + splitArgs, +} from '../common/source-utils'; + +export type MutableConfigSeverity = 'critical' | 'high' | 'medium' | 'low' | 'info'; + +/** A single tracked write to an upgrade-configuration key. */ +export interface MutableUpgradeConfigWrite { + fn: string; + key: string; + scope: 'instance' | 'persistent' | 'temporary' | 'unknown'; + line: number; + hasAuthorization: boolean; + authorizedBy?: string; +} + +export interface MutableUpgradeConfigFinding { + ruleId: string; + severity: MutableConfigSeverity; + title: string; + functionName: string; + key: string; + line: number; + hasAuthorization: boolean; + message: string; + suggestion: string; +} + +export interface MutableUpgradeConfigReport { + findings: MutableUpgradeConfigFinding[]; + writes: MutableUpgradeConfigWrite[]; + unsafeWriteCount: number; + totalWriteCount: number; + recommendations: string[]; +} + +/** Storage scopes that can carry upgrade configuration. */ +const STORAGE_SCOPE_REGEX = /\bstorage\s*\(\s*\)\s*\.\s*(instance|persistent|temporary)\s*\(\s*\)\s*\.\s*(set|put)\s*\(/g; + +/** + * Upgrade-configuration key heuristic. Matches keys that plausibly store what + * a contract may be upgraded to, delimited so `code` or `upgrade` are not + * matched as substrings of unrelated keys. + */ +const UPGRADE_CONFIG_KEY = + /(^|_)(upgrade|wasm|code|implementation|impl|target|next_hash|new_code|bytecode)(_|$)/i; + +/** Access-control primitives that would protect a configuration write. */ +const ACCESS_CONTROL = + /require_auth\s*\(|require_auth_for_args\s*\(|\b(admin|owner|admin_owner|governor|controller)\s*\.\s*require_auth\s*\(|\bonly_admin\s*\(|assert_admin\s*\(|check_admin\s*\(|ensure_admin\s*\(/; + +function findAuthorization(body: string): { has: boolean; by?: string } { + const m = ACCESS_CONTROL.exec(body); + return m + ? { has: true, by: m[0] } + : { has: false }; +} + +/** Resolve a storage-key argument to a readable key name. */ +function resolveStorageKey(raw: string): string { + const trimmed = raw.trim().replace(/^[&*]+/, ''); + const literal = trimmed.match(/"((?:[^"\\]|\\.)*)"/); + return literal ? literal[1] : trimmed; +} + +/** + * Track every write to an upgrade-configuration key, tagged with whether the + * enclosing function carries an authorization check. + */ +export function detectMutableUpgradeConfigWrites( + source: string, +): MutableUpgradeConfigWrite[] { + const masked = maskNonCode(source); + const lineOf = createLineResolver(source); + const functions = extractFunctions(masked, source); + const writes: MutableUpgradeConfigWrite[] = []; + + for (const fn of functions) { + const body = masked.slice(fn.bodyStart, fn.bodyEnd); + const auth = findAuthorization(body); + + let m: RegExpExecArray | null; + STORAGE_SCOPE_REGEX.lastIndex = 0; + while ((m = STORAGE_SCOPE_REGEX.exec(body)) !== null) { + const scope = m[1] as MutableUpgradeConfigWrite['scope']; + const offset = fn.bodyStart + m.index; + const openParen = offset + m[0].length - 1; + const argsText = extractArgs(masked, source, openParen).text; + const args = splitArgs(argsText); + const key = args.length > 0 ? resolveStorageKey(args[0]) : 'unknown'; + + if (!UPGRADE_CONFIG_KEY.test(key)) continue; + + writes.push({ + fn: fn.name, + key, + scope, + line: lineOf(offset), + hasAuthorization: auth.has, + authorizedBy: auth.by, + }); + } + } + + return writes.sort((a, b) => a.line - b.line); +} + +/** + * Analyze mutation paths for upgrade configuration in a Soroban contract. + */ +export function analyzeMutableUpgradeConfig(source: string): MutableUpgradeConfigReport { + const writes = detectMutableUpgradeConfigWrites(source); + const findings: MutableUpgradeConfigFinding[] = []; + const seen = new Set(); + + for (const w of writes) { + const dedupeKey = `${w.fn}:${w.line}:${w.key}`; + if (seen.has(dedupeKey)) continue; + seen.add(dedupeKey); + + if (!w.hasAuthorization) { + findings.push({ + ruleId: 'soroban-mutable-upgrade-config', + severity: 'high', + title: 'Unauthorized mutation of upgrade configuration', + functionName: w.fn, + key: w.key, + line: w.line, + hasAuthorization: false, + message: + `Function '${w.fn}' writes upgrade configuration key '${w.key}' (${w.scope} ` + + `storage) without an authorization check, so anyone can redirect future upgrades.`, + suggestion: + `Guard '${w.fn}' with \`admin.require_auth()\` and consider restricting which ` + + `callers may write the key, e.g. via a dedicated admin-only setter plus an event.`, + }); + } + } + + const unsafeWriteCount = writes.filter((w) => !w.hasAuthorization).length; + + const recommendations: string[] = []; + if (unsafeWriteCount > 0) { + recommendations.push( + `${unsafeWriteCount} write(s) to upgrade configuration are unauthenticated. ` + + 'Require an authorized admin for every path that can change the upgrade target.', + ); + } + if (writes.length > 0) { + recommendations.push( + 'Emit a contract event with the previous and new upgrade configuration on every write, and track the writer address.', + ); + } + + return { + findings: findings.sort((a, b) => a.line - b.line), + writes, + unsafeWriteCount, + totalWriteCount: writes.length, + recommendations, + }; +} + +export class MutableUpgradeConfigAnalyzer { + public static readonly RULE_ID = 'soroban-mutable-upgrade-config'; + + analyze(source: string): MutableUpgradeConfigReport { + return analyzeMutableUpgradeConfig(source); + } +} \ No newline at end of file diff --git a/packages/analyzers/soroban/upgrades/__tests__/upgradeability-analyzer.spec.ts b/packages/analyzers/soroban/upgrades/__tests__/upgradeability-analyzer.spec.ts new file mode 100644 index 00000000..2bea9792 --- /dev/null +++ b/packages/analyzers/soroban/upgrades/__tests__/upgradeability-analyzer.spec.ts @@ -0,0 +1,116 @@ +import { + analyzeUpgradeability, + detectUpgradeMechanisms, + UpgradeabilityAnalyzer, +} from '../upgradeability-analyzer'; + +const CONTRACT_WITH_UNPROTECTED_UPGRADE = ` +#![no_std] + +use soroban_sdk::{contract, contractimpl, token, Address, Env, Symbol, Vec}; + +#[contract] +pub struct Upgradeable; + +#[contractimpl] +impl Upgradeable { + pub fn upgrade(env: Env, new_wasm: Bytes) { + env.update_current_contract_wasm(&new_wasm); + } + + pub fn guarded_upgrade(env: Env, admin: Address, new_wasm: Bytes) { + admin.require_auth(); + env.update_current_contract_wasm(&new_wasm); + } + + pub fn read_version(env: Env) -> u32 { + 1 + } +} +`; + +const CONTRACT_WITH_DEPLOYER = ` +#![no_std] + +use soroban_sdk::{contract, contractimpl, env, Address, Env, BytesN}; + +#[contract] +pub struct Factory; + +#[contractimpl] +impl Factory { + pub fn spawn(env: Env, deployer: Address) -> BytesN<32> { + deployer.require_auth(); + let salt = BytesN::from_array(&env, &[0u8; 32]); + env.deployer().deploy_contract(&salt, &env.current_contract_address()) + } + + pub fn plain_spawn(env: Env) -> BytesN<32> { + let salt = BytesN::from_array(&env, &[0u8; 32]); + env.deployer().deploy_contract(&salt, &env.current_contract_address()) + } +} +`; + +describe('SorobanUpgradeabilityAnalyzer (#924)', () => { + it('detects wasm-replacement upgrade entry points', () => { + const entryPoints = detectUpgradeMechanisms(CONTRACT_WITH_UNPROTECTED_UPGRADE); + const wasm = entryPoints.filter((ep) => ep.mechanism === 'wasm-replacement'); + expect(wasm.length).toBeGreaterThanOrEqual(2); + expect(wasm.some((ep) => ep.functionName === 'upgrade')).toBe(true); + expect(wasm.some((ep) => ep.functionName === 'guarded_upgrade')).toBe(true); + }); + + it('reports uncontrolled upgrade entry points as critical findings', () => { + const report = analyzeUpgradeability(CONTRACT_WITH_UNPROTECTED_UPGRADE); + const uncontrolled = report.findings.filter((f) => f.title === 'Uncontrolled upgrade entry point'); + expect(uncontrolled.length).toBeGreaterThanOrEqual(1); + const upgrade = uncontrolled.find((f) => f.functionName === 'upgrade'); + expect(upgrade).toBeDefined(); + expect(upgrade?.severity).toBe('critical'); + expect(upgrade?.ruleId).toBe('soroban-upgradeability'); + expect(upgrade?.message).toContain('without an authorization check'); + }); + + it('recognizes admin.require_auth() as protecting an upgrade entry point', () => { + const report = analyzeUpgradeability(CONTRACT_WITH_UNPROTECTED_UPGRADE); + const guarded = report.entryPoints.find((ep) => ep.functionName === 'guarded_upgrade'); + expect(guarded).toBeDefined(); + expect(guarded?.hasAuthorization).toBe(true); + expect(guarded?.authorizedBy).toContain('require_auth'); + }); + + it('flags deployment via env.deployer() and distinguishes authorization', () => { + const report = analyzeUpgradeability(CONTRACT_WITH_DEPLOYER); + expect(report.upgradeMechanisms).toContain('deployer'); + const protectedSpawn = report.entryPoints.find((ep) => ep.functionName === 'spawn'); + const plainSpawn = report.entryPoints.find((ep) => ep.functionName === 'plain_spawn'); + expect(protectedSpawn?.hasAuthorization).toBe(true); + expect(plainSpawn?.hasAuthorization).toBe(false); + expect(report.hasUpgradeablePaths).toBe(true); + }); + + it('reports no upgradeable paths for a plain contract', () => { + const plain = ` + #[contractimpl] + impl Counter { + pub fn increment(env: Env, key: Symbol) -> u32 { + let n: u32 = env.storage().instance().get(&key).unwrap_or(0); + env.storage().instance().set(&key, &(n + 1)); + n + 1 + } + } + `; + const report = analyzeUpgradeability(plain); + expect(report.entryPoints).toHaveLength(0); + expect(report.hasUpgradeablePaths).toBe(false); + expect(report.findings).toHaveLength(0); + }); + + it('exposes the analyzer class with a stable rule id', () => { + expect(UpgradeabilityAnalyzer.RULE_ID).toBe('soroban-upgradeability'); + const analyzer = new UpgradeabilityAnalyzer(); + const report = analyzer.analyze(CONTRACT_WITH_UNPROTECTED_UPGRADE); + expect(report.hasUpgradeablePaths).toBe(true); + }); +}); \ No newline at end of file diff --git a/packages/analyzers/soroban/upgrades/index.ts b/packages/analyzers/soroban/upgrades/index.ts new file mode 100644 index 00000000..3eb99f50 --- /dev/null +++ b/packages/analyzers/soroban/upgrades/index.ts @@ -0,0 +1 @@ +export * from './upgradeability-analyzer'; \ No newline at end of file diff --git a/packages/analyzers/soroban/upgrades/upgradeability-analyzer.ts b/packages/analyzers/soroban/upgrades/upgradeability-analyzer.ts new file mode 100644 index 00000000..b73b8472 --- /dev/null +++ b/packages/analyzers/soroban/upgrades/upgradeability-analyzer.ts @@ -0,0 +1,267 @@ +/** + * Issue #924 — Soroban Upgradeability Analyzer + * + * Detects upgrade-related mechanisms in Soroban (Rust) contracts and reports + * the entry points through which contract behaviour can be replaced, together + * with whether each entry point is protected by an authorization check. + * + * Uncontrolled upgrades let an actor swap the deployed behaviour for a + * malicious or unintended implementation, which is a governance and security + * risk. This analyzer is lexical (no AST): comments and string literals are + * masked out via `common/source-utils` before the upgrade markers are matched. + */ + +import { + maskNonCode, + createLineResolver, + extractFunctions, +} from '../common/source-utils'; + +export type UpgradeSeverity = 'critical' | 'high' | 'medium' | 'low' | 'info'; + +export type UpgradeMechanism = + | 'wasm-replacement' + | 'implementation-swap' + | 'deployer' + | 'migration' + | 'version-switch'; + +/** A single place where an upgrade can be triggered from the contract code. */ +export interface UpgradeEntryPoint { + mechanism: UpgradeMechanism; + /** Function that contains the upgrade trigger. */ + functionName: string; + /** 1-based line of the `fn` keyword. */ + line: number; + /** The matched trigger call/expression. */ + call: string; + /** True when the enclosing function body contains an authorization check. */ + hasAuthorization: boolean; + /** The authorization primitive detected, when present. */ + authorizedBy?: string; +} + +export interface UpgradeabilityFinding { + ruleId: string; + severity: UpgradeSeverity; + title: string; + functionName: string; + mechanism: UpgradeMechanism; + message: string; + suggestion: string; + line?: number; +} + +export interface UpgradeabilityReport { + findings: UpgradeabilityFinding[]; + upgradeMechanisms: UpgradeMechanism[]; + entryPoints: UpgradeEntryPoint[]; + hasUpgradeablePaths: boolean; +} + +interface MechanismMarker { + mechanism: UpgradeMechanism; + call: string; + re: RegExp; +} + +/** Upgrade trigger markers matched inside function bodies. */ +const MECHANISM_MARKERS: MechanismMarker[] = [ + { + mechanism: 'wasm-replacement', + call: 'env.update_current_contract_wasm(...)', + re: /update_current_contract_wasm\s*\(|update_current_contract_wasm_from_contract\s*\(|set_current_wasm\s*\(/, + }, + { + mechanism: 'wasm-replacement', + call: '.set_code(...) / .set_wasm(...) / set_wasm_hash(...)', + re: /\.set_code\s*\(|\.set_wasm\s*\(|set_wasm_hash\s*\(/, + }, + { + mechanism: 'implementation-swap', + call: 'set_implementation(...)', + re: /\bset_implementation\s*\(|set_impl\s*\(|set_target\s*\(/, + }, + { + mechanism: 'deployer', + call: 'env.deployer().deploy_contract(...)', + re: /deployer\s*\(\s*\)\s*\.\s*deploy_contract\s*\(|Deployer\s*::\s*new\s*\(|soroban_sdk\s*::\s*deploy|env\s*\.\s*deployer\s*\(/, + }, + { + mechanism: 'version-switch', + call: 'version_switch(...) / forward(...)', + re: /\bversion_switch\s*\(|\bforward\s*\(|\bset_version\s*\(/, + }, + { + mechanism: 'migration', + call: 'migrate(...)', + re: /\bmigrate\s*\(|\bmigration\s*\(/, + }, +]; + +interface AuthPattern { + name: string; + re: RegExp; +} + +/** Authorization primitives recognised as protecting an upgrade entry point. */ +const AUTH_PATTERNS: AuthPattern[] = [ + { name: 'require_auth', re: /\brequire_auth\s*\(/ }, + { name: 'require_auth_for_args', re: /require_auth_for_args\s*\(/ }, + { name: 'admin/owner/controller.require_auth', re: /\b(admin|owner|admin_owner|governor|controller|keeper)\s*\.\s*require_auth\s*\(/ }, + { name: 'only_admin/assert_admin', re: /\bonly_admin\s*\(|assert_admin\s*\(|check_admin\s*\(|ensure_admin\s*\(/ }, +]; + +/** Convenience function names that are commonly attached to upgrade entry points. */ +const UPGRADE_FN_HINTS = + /\b(upgrade|update_wasm|set_wasm|set_code|set_implementation|migrate|release|update_contract|refresh_contract|redeploy|replace_contract)\b/i; + +function findAuthorization(body: string): { has: boolean; by?: string } { + for (const auth of AUTH_PATTERNS) { + if (auth.re.test(body)) { + return { has: true, by: auth.name }; + } + } + return { has: false }; +} + +/** + * Detect the upgrade mechanisms reachable from each function in a Soroban + * source file. + */ +export function detectUpgradeMechanisms(source: string): UpgradeEntryPoint[] { + const masked = maskNonCode(source); + const lineOf = createLineResolver(source); + const functions = extractFunctions(masked, source); + const entryPoints: UpgradeEntryPoint[] = []; + + for (const fn of functions) { + const body = masked.slice(fn.bodyStart, fn.bodyEnd); + const auth = findAuthorization(body); + let matchedMarker = false; + + for (const marker of MECHANISM_MARKERS) { + marker.re.lastIndex = 0; + const m = marker.re.exec(body); + if (m) { + matchedMarker = true; + const offset = fn.bodyStart + m.index; + entryPoints.push({ + mechanism: marker.mechanism, + functionName: fn.name, + line: lineOf(offset), + call: marker.call, + hasAuthorization: auth.has, + authorizedBy: auth.by, + }); + } + } + + // A function explicitly named as an upgrade entry point is an entry point + // even when the trigger is delegated (e.g. reads the target from storage). + // Skip the name-based entry point when a concrete trigger already matched, + // to avoid emitting the function twice with different mechanisms. + if (!matchedMarker && UPGRADE_FN_HINTS.test(fn.name)) { + entryPoints.push({ + mechanism: 'migration', + functionName: fn.name, + line: fn.line, + call: `fn ${fn.name}`, + hasAuthorization: auth.has, + authorizedBy: auth.by, + }); + } + } + + // Deduplicate identical (function, line, call) entry points. + const seen = new Set(); + return entryPoints.filter((ep) => { + const key = `${ep.functionName}:${ep.line}:${ep.call}`; + if (seen.has(key)) return false; + seen.add(key); + return true; + }).sort((a, b) => a.line - b.line); +} + +/** + * Analyze upgradeability of a Soroban contract. Returns findings about + * uncontrolled upgrade paths plus the tracked entry points. + */ +export function analyzeUpgradeability(source: string): UpgradeabilityReport { + const entryPoints = detectUpgradeMechanisms(source); + const findings: UpgradeabilityFinding[] = []; + const mechanisms = new Set(); + + for (const ep of entryPoints) { + mechanisms.add(ep.mechanism); + if (!ep.hasAuthorization) { + findings.push({ + ruleId: 'soroban-upgradeability', + severity: 'critical', + title: 'Uncontrolled upgrade entry point', + functionName: ep.functionName, + mechanism: ep.mechanism, + line: ep.line, + message: + `Upgrade entry point '${ep.functionName}' (${ep.call}) can be triggered without ` + + `an authorization check, so any caller can replace the deployed behaviour.`, + suggestion: + `Guard the entry point with an admin/owner check: add \`admin.require_auth()\` ` + + `(or \`require_auth_for_args\`) at the start of '${ep.functionName}', and consider ` + + `adding a timelock or multi-sig before upgradeable state can be mutated.`, + }); + } + } + + if (entryPoints.length > 0) { + const protectedCount = entryPoints.filter((ep) => ep.hasAuthorization).length; + findings.push({ + ruleId: 'soroban-upgradeability', + severity: 'medium', + title: 'Contract is upgradeable', + functionName: entryPoints[0].functionName, + mechanism: entryPoints[0].mechanism, + message: + `Contract exposes ${entryPoints.length} upgradeable path(s) (` + + `${[...mechanisms].join(', ')}); ${protectedCount} of them carry an authorization ` + + `check. Upgradeability itself is not a vulnerability but it concentrates risk.`, + suggestion: + `Document each upgrade path and who can trigger it. Prefer a timelocked ` + + `admin (e.g. Stellar governance or a multi-sig) over a single admin key, and ` + + `emit an event on every upgrade for auditability.`, + }); + } + + if (mechanisms.size > 1) { + findings.push({ + ruleId: 'soroban-upgradeability', + severity: 'low', + title: 'Multiple upgrade mechanisms present', + functionName: entryPoints[0].functionName, + mechanism: entryPoints[0].mechanism, + message: + `Multiple upgrade mechanisms (${[...mechanisms].join(', ')}) were detected. ` + + `Several ways to replace contract code increase the attack surface and make ` + + `governance harder to reason about.`, + suggestion: + `Consolidate upgrade paths behind a single guarded entry point backed by one ` + + `authorized admin/storage-controlled target.`, + }); + } + + const report = { + findings: findings.sort((a, b) => (a.line ?? Infinity) - (b.line ?? Infinity)), + upgradeMechanisms: [...mechanisms] as UpgradeMechanism[], + entryPoints, + hasUpgradeablePaths: entryPoints.length > 0, + }; + return report; +} + +export class UpgradeabilityAnalyzer { + public static readonly RULE_ID = 'soroban-upgradeability'; + + analyze(source: string): UpgradeabilityReport { + return analyzeUpgradeability(source); + } +} \ No newline at end of file diff --git a/packages/rules/soroban/src/deployment/deployment-config-rule.ts b/packages/rules/soroban/src/deployment/deployment-config-rule.ts new file mode 100644 index 00000000..3df59145 --- /dev/null +++ b/packages/rules/soroban/src/deployment/deployment-config-rule.ts @@ -0,0 +1,37 @@ +/** + * Rule family: soroban-deployment-config-* (#927) + * + * Thin Soroban rule over the deployment configuration analyzer so detection + * is surfaced through the GasGuard Soroban rule namespace. + */ +import { + analyzeDeploymentConfig, + detectDeploymentConfigFindings, + DeploymentConfigFinding, + DeploymentConfigReport, + DeploymentConfigSetting, + DeploymentSeverity, +} from '../../../../analyzers/soroban/deployment/deployment-config-analyzer'; + +export type { + DeploymentConfigFinding, + DeploymentConfigReport, + DeploymentConfigSetting, + DeploymentSeverity, +}; + +/** Return the deployment-configuration findings for a deploy config. */ +export function detectDeploymentConfigurationFindings( + configuration: string, + source?: string, +): DeploymentConfigFinding[] { + return detectDeploymentConfigFindings(configuration, source); +} + +/** Full report, including parsed settings and missing-required keys. */ +export function analyzeSorobanDeploymentConfig( + configuration: string, + source?: string, +): DeploymentConfigReport { + return analyzeDeploymentConfig(configuration, source); +} \ No newline at end of file diff --git a/packages/rules/soroban/src/deployment/index.ts b/packages/rules/soroban/src/deployment/index.ts index bf628292..d29bdf71 100644 --- a/packages/rules/soroban/src/deployment/index.ts +++ b/packages/rules/soroban/src/deployment/index.ts @@ -1 +1,2 @@ -export * from './debug-config-rule'; \ No newline at end of file +export * from './debug-config-rule'; +export * from './deployment-config-rule'; \ No newline at end of file diff --git a/packages/rules/soroban/src/index.ts b/packages/rules/soroban/src/index.ts index a671ca03..91c967c4 100644 --- a/packages/rules/soroban/src/index.ts +++ b/packages/rules/soroban/src/index.ts @@ -8,6 +8,7 @@ export * from './calls'; export * from './serialization'; export * from './events'; export * from './authorization'; +export * from './upgrades'; export * from './budget'; export * from './prioritization'; export * from './functions'; diff --git a/packages/rules/soroban/src/upgrades/index.ts b/packages/rules/soroban/src/upgrades/index.ts new file mode 100644 index 00000000..86766e1d --- /dev/null +++ b/packages/rules/soroban/src/upgrades/index.ts @@ -0,0 +1,3 @@ +export * from './upgradeability-rule'; +export * from './unprotected-upgrade-rule'; +export * from './mutable-upgrade-config-rule'; \ No newline at end of file diff --git a/packages/rules/soroban/src/upgrades/mutable-upgrade-config-rule.ts b/packages/rules/soroban/src/upgrades/mutable-upgrade-config-rule.ts new file mode 100644 index 00000000..5db214c0 --- /dev/null +++ b/packages/rules/soroban/src/upgrades/mutable-upgrade-config-rule.ts @@ -0,0 +1,34 @@ +/** + * Rule family: soroban-mutable-upgrade-config-* (#926) + * + * Thin Soroban rule over the storage analyzer for mutable upgrade + * configuration, surfaced through the Soroban rule namespace. + */ +import { + analyzeMutableUpgradeConfig, + detectMutableUpgradeConfigWrites, + MutableUpgradeConfigAnalyzer, + MutableUpgradeConfigFinding, + MutableUpgradeConfigReport, + MutableUpgradeConfigWrite, +} from '../../../../analyzers/soroban/storage/mutable-upgrade-config-analyzer'; + +export type { + MutableUpgradeConfigFinding, + MutableUpgradeConfigReport, + MutableUpgradeConfigWrite, +}; + +/** Return findings for unsafe writes to upgrade-configuration storage keys. */ +export function detectMutableUpgradeConfigFindings( + source: string, +): MutableUpgradeConfigFinding[] { + return analyzeMutableUpgradeConfig(source).findings; +} + +/** Track every write to upgrade-configuration keys and its authorization. */ +export function detectUpgradeConfigWrites(source: string): MutableUpgradeConfigWrite[] { + return detectMutableUpgradeConfigWrites(source); +} + +export { MutableUpgradeConfigAnalyzer }; \ No newline at end of file diff --git a/packages/rules/soroban/src/upgrades/unprotected-upgrade-rule.ts b/packages/rules/soroban/src/upgrades/unprotected-upgrade-rule.ts new file mode 100644 index 00000000..063c6ef3 --- /dev/null +++ b/packages/rules/soroban/src/upgrades/unprotected-upgrade-rule.ts @@ -0,0 +1,23 @@ +/** + * Rule family: soroban-unprotected-upgrade-* (#925) + * + * Thin Soroban rule over the security analyzer for unprotected upgrade + * functions, surfaced through the Soroban rule namespace. + */ +import { + analyzeUnprotectedUpgrades, + UnprotectedUpgradeAnalyzer, + UnprotectedUpgradeFinding, + UpgradeSecuritySeverity, +} from '../../../../analyzers/soroban/security/unprotected-upgrade-analyzer'; + +export type { UnprotectedUpgradeFinding, UpgradeSecuritySeverity }; + +/** Return findings for upgrade functions missing an access-control check. */ +export function detectUnprotectedUpgradeFunctions( + sourceCode: string, +): UnprotectedUpgradeFinding[] { + return analyzeUnprotectedUpgrades(sourceCode); +} + +export { UnprotectedUpgradeAnalyzer }; \ No newline at end of file diff --git a/packages/rules/soroban/src/upgrades/upgradeability-rule.ts b/packages/rules/soroban/src/upgrades/upgradeability-rule.ts new file mode 100644 index 00000000..4cf40340 --- /dev/null +++ b/packages/rules/soroban/src/upgrades/upgradeability-rule.ts @@ -0,0 +1,41 @@ +/** + * Rule family: soroban-upgradeability-* (#924) + * + * Thin Soroban rule over the upgradeability analyzer so detection is surfaced + * through the GasGuard Soroban rule namespace. + */ +import { + analyzeUpgradeability, + detectUpgradeMechanisms, + UpgradeabilityAnalyzer, + UpgradeabilityFinding, + UpgradeabilityReport, + UpgradeEntryPoint, + UpgradeMechanism, + UpgradeSeverity, +} from '../../../../analyzers/soroban/upgrades/upgradeability-analyzer'; + +export type { + UpgradeabilityFinding, + UpgradeabilityReport, + UpgradeEntryPoint, + UpgradeMechanism, + UpgradeSeverity, +}; + +/** Return the upgradeability findings for a Soroban source file. */ +export function detectUpgradeabilityFindings(source: string): UpgradeabilityFinding[] { + return analyzeUpgradeability(source).findings; +} + +/** Return the upgrade entry points tracked by the analyzer. */ +export function detectUpgradeEntryPoints(source: string): UpgradeEntryPoint[] { + return detectUpgradeMechanisms(source); +} + +/** Full report, including the tracked entry points and mechanisms. */ +export function analyzeSorobanUpgradeability(source: string): UpgradeabilityReport { + return analyzeUpgradeability(source); +} + +export { UpgradeabilityAnalyzer }; \ No newline at end of file diff --git a/packages/rules/soroban/tests/deployment-config.spec.ts b/packages/rules/soroban/tests/deployment-config.spec.ts new file mode 100644 index 00000000..37c05a8a --- /dev/null +++ b/packages/rules/soroban/tests/deployment-config.spec.ts @@ -0,0 +1,50 @@ +import { + detectDeploymentConfigurationFindings, + analyzeSorobanDeploymentConfig, +} from '../src/deployment/deployment-config-rule'; + +describe('Soroban Deployment Configuration Rules (#927)', () => { + const VALID = ` +network = "mainnet" +rpc_url = "https://rpc.stellar.org" +owner = "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +wasm_hash = "b9f0b8cbe0b79bca" +`; + + const BROKEN = ` +network = "rinkeby" +owner = "bob" +max_fee = "lots" +unknown_option = true +`; + + test('detectDeploymentConfigurationFindings passes a valid config', () => { + const findings = detectDeploymentConfigurationFindings(VALID); + expect(findings).toHaveLength(0); + }); + + test('detectDeploymentConfigurationFindings flags invalid settings and gaps', () => { + const findings = detectDeploymentConfigurationFindings(BROKEN); + const network = findings.find((f) => f.key === 'network'); + expect(network).toBeDefined(); + expect(network?.ruleId).toBe('soroban-deployment-config'); + expect(network?.severity).toBe('high'); + + const maxFee = findings.find((f) => f.key === 'max_fee'); + expect(maxFee?.severity).toBe('medium'); + + const rpcMissing = findings.find((f) => f.key === 'rpc_url' && f.title.includes('Missing')); + expect(rpcMissing).toBeDefined(); + + const unknown = findings.find((f) => f.key === 'unknown_option'); + expect(unknown).toBeDefined(); + expect(unknown?.severity).toBe('low'); + }); + + test('analyzeSorobanDeploymentConfig reports parsed settings and validity', () => { + const report = analyzeSorobanDeploymentConfig(VALID); + expect(report.valid).toBe(true); + expect(report.network).toBe('mainnet'); + expect(report.rpcUrl).toBe('https://rpc.stellar.org'); + }); +}); \ No newline at end of file diff --git a/packages/rules/soroban/tests/mutable-upgrade-config.spec.ts b/packages/rules/soroban/tests/mutable-upgrade-config.spec.ts new file mode 100644 index 00000000..74ed0d84 --- /dev/null +++ b/packages/rules/soroban/tests/mutable-upgrade-config.spec.ts @@ -0,0 +1,42 @@ +import { + detectMutableUpgradeConfigFindings, + detectUpgradeConfigWrites, + MutableUpgradeConfigAnalyzer, +} from '../src/upgrades/mutable-upgrade-config-rule'; + +describe('Mutable Soroban Upgrade Configuration Rules (#926)', () => { + const SAMPLE = ` + #[contractimpl] + impl UpgradeableImpl { + pub fn set_wasm_hash(env: Env, wasm_hash: BytesN<32>) { + env.storage().persistent().set(&Symbol::new(&env, "wasm_hash"), &wasm_hash); + } + + pub fn set_target(env: Env, admin: Address, target: BytesN<32>) { + admin.require_auth(); + env.storage().instance().set(&Symbol::new(&env, "next_wasm"), &target); + } + } + `; + + test('detectUpgradeConfigWrites tracks both config writes', () => { + const writes = detectUpgradeConfigWrites(SAMPLE); + expect(writes).toHaveLength(2); + expect(writes.map((w) => w.key).sort()).toEqual(['next_wasm', 'wasm_hash']); + }); + + test('detectMutableUpgradeConfigFindings flags only the unauthored write', () => { + const findings = detectMutableUpgradeConfigFindings(SAMPLE); + expect(findings).toHaveLength(1); + expect(findings[0].functionName).toBe('set_wasm_hash'); + expect(findings[0].severity).toBe('high'); + expect(findings[0].ruleId).toBe('soroban-mutable-upgrade-config'); + }); + + test('the analyzer class is exposed through the rule namespace', () => { + expect(MutableUpgradeConfigAnalyzer.RULE_ID).toBe('soroban-mutable-upgrade-config'); + const report = new MutableUpgradeConfigAnalyzer().analyze(SAMPLE); + expect(report.totalWriteCount).toBe(2); + expect(report.unsafeWriteCount).toBe(1); + }); +}); \ No newline at end of file diff --git a/packages/rules/soroban/tests/unprotected-upgrade.spec.ts b/packages/rules/soroban/tests/unprotected-upgrade.spec.ts new file mode 100644 index 00000000..85f9b577 --- /dev/null +++ b/packages/rules/soroban/tests/unprotected-upgrade.spec.ts @@ -0,0 +1,34 @@ +import { + detectUnprotectedUpgradeFunctions, + UnprotectedUpgradeAnalyzer, +} from '../src/upgrades/unprotected-upgrade-rule'; + +describe('Unprotected Soroban Upgrade Function Rules (#925)', () => { + const SAMPLE = ` + #[contractimpl] + impl UpgradeableImpl { + pub fn upgrade(env: Env, new_wasm: Bytes) { + env.update_current_contract_wasm(&new_wasm); + } + + pub fn guarded_upgrade(env: Env, admin: Address, new_wasm: Bytes) { + admin.require_auth(); + env.update_current_contract_wasm(&new_wasm); + } + } + `; + + test('detectUnprotectedUpgradeFunctions reports the unguarded upgrade only', () => { + const findings = detectUnprotectedUpgradeFunctions(SAMPLE); + expect(findings).toHaveLength(1); + expect(findings[0].functionName).toBe('upgrade'); + expect(findings[0].severity).toBe('critical'); + expect(findings[0].ruleId).toBe('soroban-unprotected-upgrade'); + }); + + test('the analyzer class is exposed through the rule namespace', () => { + expect(UnprotectedUpgradeAnalyzer.RULE_ID).toBe('soroban-unprotected-upgrade'); + const findings = new UnprotectedUpgradeAnalyzer().analyze(SAMPLE); + expect(findings.some((f) => f.functionName === 'guarded_upgrade')).toBe(false); + }); +}); \ No newline at end of file diff --git a/packages/rules/soroban/tests/upgradeability.spec.ts b/packages/rules/soroban/tests/upgradeability.spec.ts new file mode 100644 index 00000000..01eab17d --- /dev/null +++ b/packages/rules/soroban/tests/upgradeability.spec.ts @@ -0,0 +1,58 @@ +import { + detectUpgradeabilityFindings, + analyzeSorobanUpgradeability, + UpgradeabilityAnalyzer, +} from '../src/upgrades/upgradeability-rule'; + +describe('Soroban Upgradeability Rules (#924)', () => { + const GUARDED = ` + #[contractimpl] + impl UpgradeableImpl { + pub fn upgrade(env: Env, admin: Address, new_wasm: Bytes) { + admin.require_auth(); + env.update_current_contract_wasm(&new_wasm); + } + } + `; + + const UNGUARDED = ` + #[contractimpl] + impl UpgradeableImpl { + pub fn upgrade(env: Env, new_wasm: Bytes) { + env.update_current_contract_wasm(&new_wasm); + } + } + `; + + test('detectUpgradeabilityFindings flags unguarded upgrade entry points', () => { + const findings = detectUpgradeabilityFindings(UNGUARDED); + const uncontrolled = findings.filter( + (f) => f.ruleId === 'soroban-upgradeability' && f.title === 'Uncontrolled upgrade entry point', + ); + expect(uncontrolled.length).toBe(1); + expect(uncontrolled[0].severity).toBe('critical'); + expect(uncontrolled[0].functionName).toBe('upgrade'); + }); + + test('guarded upgrade paths do not produce uncontrolled findings', () => { + const findings = detectUpgradeabilityFindings(GUARDED); + const uncontrolled = findings.filter((f) => f.title === 'Uncontrolled upgrade entry point'); + expect(uncontrolled).toHaveLength(0); + // The "contract is upgradeable" informational finding is still emitted. + expect(findings.some((f) => f.title === 'Contract is upgradeable')).toBe(true); + }); + + test('analyzeSorobanUpgradeability returns entry points and mechanisms', () => { + const report = analyzeSorobanUpgradeability(GUARDED); + expect(report.hasUpgradeablePaths).toBe(true); + expect(report.upgradeMechanisms).toContain('wasm-replacement'); + expect(report.entryPoints[0].hasAuthorization).toBe(true); + expect(report.entryPoints[0].authorizedBy).toContain('require_auth'); + }); + + test('exposes the analyzer class through the rule namespace', () => { + expect(UpgradeabilityAnalyzer.RULE_ID).toBe('soroban-upgradeability'); + const report = new UpgradeabilityAnalyzer().analyze(GUARDED); + expect(report.hasUpgradeablePaths).toBe(true); + }); +}); \ No newline at end of file