From e0803d18ba199dec9b894ccf3ecfe93f68ecd3bd Mon Sep 17 00:00:00 2001 From: Joeloo1 Date: Tue, 2 Jun 2026 13:51:31 +0100 Subject: [PATCH 1/2] feat: detect weak role hierarchies in Soroban contracts Closes #358 --- .../detect-weak-role-hierarchies.ts | 79 +++++++++++++ .../detect-weak-role-hierarchies.spec.ts | 106 ++++++++++++++++++ .../fixtures/stellar-weak-role-hierarchy.json | 25 +++++ 3 files changed, 210 insertions(+) create mode 100644 rules/stellar/access-control/detect-weak-role-hierarchies.ts create mode 100644 tests/rules/detect-weak-role-hierarchies.spec.ts create mode 100644 tests/rules/fixtures/stellar-weak-role-hierarchy.json diff --git a/rules/stellar/access-control/detect-weak-role-hierarchies.ts b/rules/stellar/access-control/detect-weak-role-hierarchies.ts new file mode 100644 index 0000000..ea9b377 --- /dev/null +++ b/rules/stellar/access-control/detect-weak-role-hierarchies.ts @@ -0,0 +1,79 @@ +/** + * Detect Weak Role Hierarchies in Soroban Contracts + * Flags role-assignment functions that lack a superior-authority check, + * enabling any authenticated caller to escalate privileges. + */ + +export interface WeakRoleHierarchyResult { + detected: boolean; + weakRoles: string[]; + message: string; + suggestion: string; +} + +// Functions that grant, assign, or promote roles +const ROLE_ASSIGN_PATTERNS = [ + /fn\s+(grant_role|assign_role|set_role|add_role|promote|escalate_role|make_admin|add_admin|give_admin|grant_admin|set_admin|transfer_role)\s*\(/g, +]; + +// A proper hierarchy guard verifies the CALLER holds a superior role before granting any role +const HIERARCHY_GUARD = + /admin\.require_auth|only_admin|assert_admin|is_admin\(|require_role\s*\(\s*Role::Admin|env\.require_auth_for_admin|check_admin/; + +// Basic auth present but no hierarchy check — caller identity confirmed but tier not verified +const WEAK_AUTH_GUARD = /require_auth/; + +// Extract exactly one function body starting at startIdx by counting braces, +// avoiding false positives from adjacent function definitions. +function extractFunctionBody(code: string, startIdx: number): string { + let depth = 0; + let opened = false; + for (let i = startIdx; i < code.length; i++) { + if (code[i] === '{') { depth++; opened = true; } + else if (code[i] === '}') { + depth--; + if (opened && depth === 0) return code.slice(startIdx, i + 1); + } + } + return code.slice(startIdx, startIdx + 400); +} + +export function detectWeakRoleHierarchies(code: string): WeakRoleHierarchyResult { + const weakRoles: string[] = []; + + for (const pattern of ROLE_ASSIGN_PATTERNS) { + for (const match of code.matchAll(pattern)) { + const fnName = match[1]; + const fnBody = extractFunctionBody(code, match.index ?? 0); + + // Safe: a superior-authority guard validates the caller's role tier + if (HIERARCHY_GUARD.test(fnBody)) continue; + + // Weak: only basic auth (any authenticated user can assign roles) + // or no auth at all — both allow privilege escalation + weakRoles.push(fnName); + } + } + + if (weakRoles.length === 0) { + return { + detected: false, + weakRoles: [], + message: 'Role hierarchy properly enforced on all role-assignment functions.', + suggestion: '', + }; + } + + const hasWeakAuth = WEAK_AUTH_GUARD.test(code); + const escalationType = hasWeakAuth + ? 'only basic require_auth (any authenticated user can assign roles)' + : 'no authentication at all'; + + return { + detected: true, + weakRoles, + message: `Weak role hierarchy detected in: ${weakRoles.join(', ')}. Role-assignment functions use ${escalationType}.`, + suggestion: + 'Guard every role-assignment function with a superior-authority check such as `admin.require_auth()` or `assert_admin(&env)` before modifying any role.', + }; +} diff --git a/tests/rules/detect-weak-role-hierarchies.spec.ts b/tests/rules/detect-weak-role-hierarchies.spec.ts new file mode 100644 index 0000000..346d7f3 --- /dev/null +++ b/tests/rules/detect-weak-role-hierarchies.spec.ts @@ -0,0 +1,106 @@ +import { detectWeakRoleHierarchies } from '../../rules/stellar/access-control/detect-weak-role-hierarchies'; +import { FixtureLoader } from '../../libs/testing/src/fixture-loader'; + +describe('detectWeakRoleHierarchies', () => { + describe('detection cases', () => { + it('flags a grant_role function with only require_auth', () => { + const code = ` + pub fn grant_role(env: Env, caller: Address, target: Address, role: Role) { + caller.require_auth(); + env.storage().instance().set(&target, &role); + } + `; + const result = detectWeakRoleHierarchies(code); + expect(result.detected).toBe(true); + expect(result.weakRoles).toContain('grant_role'); + expect(result.message).toMatch(/grant_role/); + }); + + it('flags add_admin with no auth guard at all', () => { + const code = ` + pub fn add_admin(env: Env, new_admin: Address) { + env.storage().instance().set(&new_admin, &Role::Admin); + } + `; + const result = detectWeakRoleHierarchies(code); + expect(result.detected).toBe(true); + expect(result.weakRoles).toContain('add_admin'); + }); + + it('flags multiple weak role functions', () => { + const code = ` + pub fn grant_role(env: Env, caller: Address, target: Address, role: Role) { + caller.require_auth(); + env.storage().instance().set(&target, &role); + } + pub fn set_role(env: Env, caller: Address, target: Address, role: Role) { + caller.require_auth(); + env.storage().instance().set(&target, &role); + } + `; + const result = detectWeakRoleHierarchies(code); + expect(result.detected).toBe(true); + expect(result.weakRoles).toHaveLength(2); + expect(result.weakRoles).toContain('grant_role'); + expect(result.weakRoles).toContain('set_role'); + }); + }); + + describe('safe cases', () => { + it('does not flag a function guarded with admin.require_auth', () => { + const code = ` + pub fn grant_role(env: Env, admin: Address, target: Address, role: Role) { + admin.require_auth(); + assert_admin(&env, &admin); + env.storage().instance().set(&target, &role); + } + `; + const result = detectWeakRoleHierarchies(code); + expect(result.detected).toBe(false); + expect(result.weakRoles).toHaveLength(0); + }); + + it('does not flag when no role-assignment functions exist', () => { + const code = ` + pub fn get_balance(env: Env, address: Address) -> i128 { + env.storage().instance().get(&address).unwrap_or(0) + } + `; + const result = detectWeakRoleHierarchies(code); + expect(result.detected).toBe(false); + }); + + it('does not flag a function guarded with only_admin', () => { + const code = ` + pub fn add_admin(env: Env, caller: Address, new_admin: Address) { + only_admin(&env, &caller); + env.storage().instance().set(&new_admin, &Role::Admin); + } + `; + const result = detectWeakRoleHierarchies(code); + expect(result.detected).toBe(false); + }); + }); + + describe('fixture validation', () => { + it('fixture matches expected structure', () => { + const fixture = FixtureLoader.loadFixture( + './tests/rules/fixtures/stellar-weak-role-hierarchy.json' + ); + expect(fixture.id).toBe('stellar-weak-role-hierarchy-1'); + expect(fixture.expectedViolations).toHaveLength(2); + expect(fixture.metadata?.category).toBe('access-control'); + }); + + it('detector agrees with fixture violations', () => { + const fixture = FixtureLoader.loadFixture( + './tests/rules/fixtures/stellar-weak-role-hierarchy.json' + ); + const result = detectWeakRoleHierarchies(fixture.input); + expect(result.detected).toBe(true); + expect(result.weakRoles).toContain('grant_role'); + expect(result.weakRoles).toContain('add_admin'); + expect(result.weakRoles).not.toContain('safe_promote'); + }); + }); +}); diff --git a/tests/rules/fixtures/stellar-weak-role-hierarchy.json b/tests/rules/fixtures/stellar-weak-role-hierarchy.json new file mode 100644 index 0000000..dd4aff3 --- /dev/null +++ b/tests/rules/fixtures/stellar-weak-role-hierarchy.json @@ -0,0 +1,25 @@ +{ + "id": "stellar-weak-role-hierarchy-1", + "name": "Weak Role Hierarchy Detection", + "description": "Flags role-assignment functions that use only basic require_auth without a superior-authority check, enabling privilege escalation", + "input": "use soroban_sdk::{contract, contractimpl, contracttype, Address, Env};\n\n#[contracttype]\npub enum Role { Admin, Moderator, User }\n\n#[contractimpl]\nimpl AccessContract {\n pub fn grant_role(env: Env, caller: Address, target: Address, role: Role) {\n caller.require_auth();\n env.storage().instance().set(&target, &role);\n }\n\n pub fn add_admin(env: Env, caller: Address, new_admin: Address) {\n caller.require_auth();\n env.storage().instance().set(&new_admin, &Role::Admin);\n }\n\n pub fn safe_promote(env: Env, admin: Address, target: Address, role: Role) {\n admin.require_auth();\n assert_admin(&env, &admin);\n env.storage().instance().set(&target, &role);\n }\n}", + "expectedViolations": [ + { + "rule_name": "detect-weak-role-hierarchies", + "severity": "High", + "message_pattern": "grant_role", + "line_number": 8 + }, + { + "rule_name": "detect-weak-role-hierarchies", + "severity": "High", + "message_pattern": "add_admin", + "line_number": 13 + } + ], + "metadata": { + "language": "soroban", + "category": "access-control", + "tags": ["role-hierarchy", "privilege-escalation", "access-control"] + } +} From fb3ce27a24ba70c0db4dd78beefd6e366f2143b0 Mon Sep 17 00:00:00 2001 From: Joeloo1 Date: Tue, 2 Jun 2026 14:02:29 +0100 Subject: [PATCH 2/2] feat: detect excessive Soroban event topics --- .../events/detect-excessive-event-topics.ts | 105 ++++++++++++++++++ .../detect-excessive-event-topics.spec.ts | 94 ++++++++++++++++ .../stellar-excessive-event-topics.json | 25 +++++ 3 files changed, 224 insertions(+) create mode 100644 rules/stellar/events/detect-excessive-event-topics.ts create mode 100644 tests/rules/detect-excessive-event-topics.spec.ts create mode 100644 tests/rules/fixtures/stellar-excessive-event-topics.json diff --git a/rules/stellar/events/detect-excessive-event-topics.ts b/rules/stellar/events/detect-excessive-event-topics.ts new file mode 100644 index 0000000..cf7299e --- /dev/null +++ b/rules/stellar/events/detect-excessive-event-topics.ts @@ -0,0 +1,105 @@ +/** + * Detect Excessive Soroban Event Topics + * Flags events whose topic lists exceed the recommended count or contain + * oversized payload types, both of which inflate execution costs. + */ + +export interface ExcessiveEventTopicsResult { + detected: boolean; + violations: EventViolation[]; + message: string; + suggestion: string; +} + +export interface EventViolation { + topicCount: number; + hasLargePayload: boolean; + snippet: string; +} + +// Soroban recommends no more than 4 topics per event +const MAX_TOPICS = 4; + +// Topics embedding large types cost more execution/storage +const LARGE_PAYLOAD_PATTERN = /\b(?:Bytes|BytesN|String|Vec|Map)\b/; + +// Locates the opening paren of every publish() call +const PUBLISH_CALL = /env\.events\(\)\.publish\s*\(/g; + +// Extract the inner content of the first balanced (...) starting at `pos` +function extractBalanced(code: string, pos: number): string | null { + // Advance past optional whitespace to find '(' + while (pos < code.length && code[pos] !== '(') pos++; + if (pos >= code.length) return null; + pos++; // skip '(' + let depth = 1; + const start = pos; + while (pos < code.length) { + if (code[pos] === '(') depth++; + else if (code[pos] === ')') { + depth--; + if (depth === 0) return code.slice(start, pos); + } + pos++; + } + return null; +} + +function countTopics(topicList: string): number { + if (topicList.trim() === '') return 0; + let depth = 0; + let count = 1; + for (const ch of topicList) { + if (ch === '(' || ch === '<') depth++; + else if (ch === ')' || ch === '>') depth--; + else if (ch === ',' && depth === 0) count++; + } + return count; +} + +export function detectExcessiveEventTopics(code: string): ExcessiveEventTopicsResult { + const violations: EventViolation[] = []; + + for (const match of code.matchAll(PUBLISH_CALL)) { + // Position just after the opening '(' of publish(; extractBalanced will + // advance to the next '(' which is the start of the topics tuple. + const afterOpen = (match.index ?? 0) + match[0].length; + + // First argument to publish() is the topics tuple: (topic1, topic2, ...) + const topicsTuple = extractBalanced(code, afterOpen); + if (topicsTuple === null) continue; + + const topicCount = countTopics(topicsTuple); + const hasLargePayload = LARGE_PAYLOAD_PATTERN.test(topicsTuple); + + if (topicCount > MAX_TOPICS || hasLargePayload) { + violations.push({ + topicCount, + hasLargePayload, + snippet: match[0].slice(0, 80), + }); + } + } + + if (violations.length === 0) { + return { + detected: false, + violations: [], + message: 'No excessive event topics detected.', + suggestion: '', + }; + } + + const reasons: string[] = []; + if (violations.some((v) => v.topicCount > MAX_TOPICS)) + reasons.push(`topic count exceeds ${MAX_TOPICS}`); + if (violations.some((v) => v.hasLargePayload)) + reasons.push('topics contain large payload types (Bytes, String, Vec, Map)'); + + return { + detected: true, + violations, + message: `Excessive event topics in ${violations.length} event(s): ${reasons.join('; ')}.`, + suggestion: `Limit topics to ${MAX_TOPICS} or fewer and avoid embedding Bytes, String, Vec, or Map directly in topics — move bulk data to the event data argument instead.`, + }; +} diff --git a/tests/rules/detect-excessive-event-topics.spec.ts b/tests/rules/detect-excessive-event-topics.spec.ts new file mode 100644 index 0000000..ed061a1 --- /dev/null +++ b/tests/rules/detect-excessive-event-topics.spec.ts @@ -0,0 +1,94 @@ +import { detectExcessiveEventTopics } from '../../rules/stellar/events/detect-excessive-event-topics'; +import { FixtureLoader } from '../../libs/testing/src/fixture-loader'; + +describe('detectExcessiveEventTopics', () => { + describe('topic count violations', () => { + it('flags an event with 5 topics', () => { + const code = `env.events().publish((a, b, c, d, e), data)`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(true); + expect(result.violations[0].topicCount).toBe(5); + expect(result.message).toMatch(/topic count exceeds 4/); + }); + + it('does not flag an event with exactly 4 topics', () => { + const code = `env.events().publish((a, b, c, d), data)`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(false); + }); + }); + + describe('large payload type violations', () => { + it('flags Bytes in the topics tuple', () => { + const code = `env.events().publish((symbol_short!("log"), Bytes::from_slice(&env, &[1,2,3])), ())`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(true); + expect(result.violations[0].hasLargePayload).toBe(true); + expect(result.message).toMatch(/large payload types/); + }); + + it('flags String in the topics tuple', () => { + const code = `env.events().publish((symbol_short!("mint"), String::from_str(&env, "x")), amt)`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(true); + expect(result.violations[0].hasLargePayload).toBe(true); + }); + + it('flags Vec in the topics tuple', () => { + const code = `env.events().publish((symbol_short!("info"), Vec::new(&env)), data)`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(true); + expect(result.violations[0].hasLargePayload).toBe(true); + }); + }); + + describe('safe cases', () => { + it('does not flag 2 plain topics', () => { + const code = `env.events().publish((symbol_short!("mint"), addr), amount)`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(false); + expect(result.violations).toHaveLength(0); + }); + + it('does not flag code with no event calls', () => { + const code = `let x = 1 + 1;`; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(false); + }); + }); + + describe('multiple events', () => { + it('reports all violating events in one result', () => { + const code = ` + env.events().publish((a, b, c, d, e), data); + env.events().publish((symbol_short!("log"), Bytes::from_slice(&env, &[])), ()); + env.events().publish((symbol_short!("ok"), addr), ()); + `; + const result = detectExcessiveEventTopics(code); + expect(result.detected).toBe(true); + expect(result.violations).toHaveLength(2); + }); + }); + + describe('fixture validation', () => { + it('fixture matches expected structure', () => { + const fixture = FixtureLoader.loadFixture( + './tests/rules/fixtures/stellar-excessive-event-topics.json' + ); + expect(fixture.id).toBe('stellar-excessive-event-topics-1'); + expect(fixture.expectedViolations).toHaveLength(2); + expect(fixture.metadata?.category).toBe('events'); + }); + + it('detector agrees with fixture violations', () => { + const fixture = FixtureLoader.loadFixture( + './tests/rules/fixtures/stellar-excessive-event-topics.json' + ); + const result = detectExcessiveEventTopics(fixture.input); + expect(result.detected).toBe(true); + expect(result.violations).toHaveLength(2); + expect(result.violations.some((v) => v.topicCount > 4)).toBe(true); + expect(result.violations.some((v) => v.hasLargePayload)).toBe(true); + }); + }); +}); diff --git a/tests/rules/fixtures/stellar-excessive-event-topics.json b/tests/rules/fixtures/stellar-excessive-event-topics.json new file mode 100644 index 0000000..d229f48 --- /dev/null +++ b/tests/rules/fixtures/stellar-excessive-event-topics.json @@ -0,0 +1,25 @@ +{ + "id": "stellar-excessive-event-topics-1", + "name": "Excessive Soroban Event Topics Detection", + "description": "Flags event publish calls whose topic tuple exceeds 4 entries or embeds large payload types (Bytes, String, Vec, Map)", + "input": "use soroban_sdk::{contract, contractimpl, Address, Env, Symbol, Bytes, symbol_short};\n\n#[contractimpl]\nimpl EventContract {\n pub fn transfer(env: Env, from: Address, to: Address, amount: i128) {\n // Excessive topic count (5)\n env.events().publish((symbol_short!(\"transfer\"), from.clone(), to.clone(), amount, amount), ());\n\n // Large payload type in topics\n env.events().publish((symbol_short!(\"log\"), Bytes::from_slice(&env, &[1, 2, 3])), ());\n\n // Safe: 2 plain topics\n env.events().publish((symbol_short!(\"ok\"), from), ());\n }\n}", + "expectedViolations": [ + { + "rule_name": "detect-excessive-event-topics", + "severity": "Warning", + "message_pattern": "topic count exceeds 4", + "line_number": 7 + }, + { + "rule_name": "detect-excessive-event-topics", + "severity": "Warning", + "message_pattern": "large payload types", + "line_number": 10 + } + ], + "metadata": { + "language": "soroban", + "category": "events", + "tags": ["events", "gas-optimization", "event-topics"] + } +}