Building Secure Contracts Skill
Overview
This skill applies systematic security analysis to smart contracts and secure API contracts. The core principle: every state mutation must be proven safe through invariant verification before an external call executes. It covers both EVM (Solidity) and Solana (Rust) ecosystems with platform-specific vulnerability patterns.
Vulnerability taxonomy: OpenSCV (94 classified security issues) Critical patterns: CEI, reentrancy guards, access modifiers, SafeMath equivalents Risk landscape: $1.8B+ in DeFi exploits Q3 2025 (access control: $953M, reentrancy: $420M)
When to Use
-
Before deploying any smart contract to mainnet
-
When auditing existing contracts for security vulnerabilities
-
When reviewing API contracts for invariant violations
-
When adding new entry points or external calls to existing contracts
-
When upgrading proxy contracts (storage slot collision risk)
-
When integrating oracles, flash loans, or third-party DeFi protocols
Iron Laws
-
NEVER make external calls before updating state — Checks-Effects-Interactions (CEI) is non-negotiable; any external call before state update is a reentrancy vector regardless of perceived safety.
-
NEVER assume access control is correct without reading every modifier — access control failures account for ~53% of 2024 DeFi losses; verify every onlyOwner , onlyRole , and custom guard.
-
NEVER trust integer arithmetic without explicit bounds checking — Solidity 0.8+ has native overflow protection but custom assembly, unchecked blocks, and Rust/Solana code require explicit verification.
-
ALWAYS enumerate all contract invariants before analysis — invariants are the ground truth for correctness; a violation is always a bug; document them in NatSpec before reviewing the implementation.
-
ALWAYS test reentrancy across full call chains, not just single functions — cross-function reentrancy (withdraw + transfer sharing state) is as dangerous as direct reentrancy.
Phase 1: Contract Reconnaissance
Goal: Map the attack surface before deep analysis.
Steps
-
Enumerate entry points: All external/public functions, fallback, receive
-
Identify state-mutating functions: Functions that modify storage
-
Map access control boundaries: Roles, modifiers, ownership checks
-
Catalog external calls: call() , transfer() , ERC20 hooks, interface calls
-
Identify trust boundaries: User input, oracle feeds, cross-contract calls
Output Format
Contract Reconnaissance
Entry Points
-
withdraw(uint256 amount)— external, state-mutating, calls msg.sender -
deposit()— payable, updates balances mapping
Access Control Map
-
onlyOwner: [list of functions] -
onlyRole(ADMIN_ROLE): [list of functions] - No modifier (verify intent): [list of functions]
External Calls
-
msg.sender.call{value: amount}("")at withdraw():L45 -
token.transferFrom(...)at deposit():L23
Trust Boundaries
- User-supplied amount at withdraw():L40
- Oracle price feed at getPrice():L67 — manipulation risk
Phase 2: Reentrancy Analysis
Goal: Identify all reentrancy vectors (direct, cross-function, read-only).
Checks-Effects-Interactions Verification
For each function with external calls:
Function: withdraw(uint256 amount)
CEI Order Analysis
- L40: CHECK — require(balances[msg.sender] >= amount) ✓
- L45: EXTERNAL CALL — msg.sender.call{value: amount}("") ← VIOLATION
- L48: EFFECT — balances[msg.sender] -= amount ← STATE AFTER CALL
FINDING: Classic reentrancy — balance updated after external call. Fix: Move L48 before L45 (CEI pattern) Severity: Critical
Fixed Pattern
require(balances[msg.sender] >= amount);
balances[msg.sender] -= amount; // Effect BEFORE external call
(bool success, ) = msg.sender.call{value: amount}("");
require(success);
Cross-Function Reentrancy Check
Identify shared state between functions that both make external calls:
Shared State: balances mapping
- withdraw() reads + writes balances + makes external call
- emergencyWithdraw() reads + writes balances + makes external call RISK: Reentrancy from withdraw() into emergencyWithdraw() bypasses checks
Phase 3: Access Control Audit
Goal: Verify every state-mutating function has appropriate guards.
Access Control Checklist
For each function:
Function Audit: updateTreasury(address newTreasury)
- Has access modifier? → NO ← FINDING: Missing onlyOwner
- Modifier verified in contract? → N/A (not present)
- Owner transferable safely? → N/A
- Time lock for critical changes? → NO
Severity: Critical — anyone can redirect protocol treasury
Fix: Add onlyOwner modifier and time-lock for parameter changes
Role Confusion Patterns
Role Check: PAUSER_ROLE vs ADMIN_ROLE
- pause() requires: PAUSER_ROLE
- unpause() requires: PAUSER_ROLE (RISK: pauser can also unpause)
- grantRole() requires: ADMIN_ROLE
Issue: Pauser can unilaterally pause and unpause — should require separate roles Severity: Medium
Phase 4: Integer Arithmetic Analysis
Goal: Identify overflow, underflow, precision loss, and rounding direction bugs.
Arithmetic Boundary Analysis
Function: calculateReward(uint256 principal, uint256 rate)
- L88:
uint256 reward = principal * rate / 1e18- Multiplication before division: OK (avoids precision loss)
- Overflow check: principal * rate could overflow if both > sqrt(uint256.max)
- Rounding: truncates toward zero — check if favors protocol or user
uncheckedblock? → NO → Solidity 0.8+ protects this
Unchecked Block Analysis
- L102-108:
unchecked { ... }- Why unchecked? Check comment and verify mathematician's claim
- Is the claimed impossibility of overflow actually proven?
- [UNVERIFIED] claim: "amount < balance guarantees no underflow"
Phase 5: Invariant Verification
Goal: Identify and verify all contract-level invariants.
Contract Invariants: LiquidityPool
- Solvency: sum(balances) == address(this).balance — [VERIFIED L90]
- Total supply: totalSupply == sum(all user shares) — [UNVERIFIED]
- Fee bound: fee <= MAX_FEE (1000 bps) — [VERIFIED by require at L45]
- Non-zero denominator: totalSupply > 0 before share calculation — [VIOLATED at L67, division-by-zero risk on first deposit]
Invariant Violation Findings
FINDING: Invariant 4 violated — first depositor can cause division by zero
- Location: L67
shares = amount * totalSupply / totalAssets - When: totalSupply == 0 on first deposit
- Impact: DoS attack on first deposit; protocol initialization blocked
- Fix: Handle zero totalSupply case separately with initial share ratio
Output: Security Report
Security Report: [Contract Name]
Summary
- Functions analyzed: N
- Findings: N (Critical: X, High: Y, Medium: Z, Low: W)
- Invariants verified: N of M
- CEI violations: N
Critical Findings
[F-01] Reentrancy in withdraw()
- Location:
src/Pool.sol:L45 - Pattern: External call before state update (CEI violation)
- Impact: Complete fund drainage
- Fix: Apply CEI pattern — update state before external call
- 5 Whys: [root cause chain]
Invariant Status
| Invariant | Status | Evidence |
|---|---|---|
| sum(balances) == balance | VERIFIED | L90 invariant check |
| totalSupply == sum(shares) | UNVERIFIED | No test coverage |
Recommendations
- [Critical] Fix reentrancy in withdraw() before deployment
- [High] Add reentrancy guard as defense-in-depth
- [Medium] Add formal invariant tests via Foundry invariant suite
Integration with Agent-Studio
Recommended Workflow
-
Invoke audit-context-building for initial code reconnaissance
-
Invoke building-secure-contracts for contract-specific analysis
-
Feed findings into security-architect for threat modeling
-
Use static-analysis (Semgrep/CodeQL) for automated confirmation
-
Use medusa-security for fuzzing-based invariant testing
Complementary Skills
Skill Relationship
audit-context-building
Builds initial mental model before contract analysis
security-architect
Consumes findings for threat modeling and STRIDE
static-analysis
Automated SAST confirmation of manual findings
medusa-security
Fuzzing and property-based testing for invariants
variant-analysis
Finds similar vulnerability patterns across codebase
web3-expert
Solidity/Ethereum ecosystem expertise
Anti-Patterns
Anti-Pattern Why It Fails Correct Approach
Auditing only the happy path Reentrancy and access control bugs are invisible in happy path Explicitly trace every error path and external call
Trusting function name for access control onlyAdmin() might not check the actual admin role Read the modifier implementation, not just its name
Assuming Solidity 0.8 prevents all integer bugs unchecked blocks, assembly, and casting bypass protection Audit all unchecked blocks and type casts explicitly
Skipping cross-function reentrancy Cross-function reentrancy bypasses single-function guards Map shared state across ALL functions making external calls
Leaving invariants implicit Unwritten invariants are unverified risks Document every invariant in NatSpec before analysis
Memory Protocol
Before starting: Check .claude/context/memory/learnings.md for prior contract audits of the same protocol or token standard.
During analysis: Write incremental findings to context report as discovered. Do not wait until the end.
After completion: Record key findings and patterns to .claude/context/memory/learnings.md . Record architecture decisions (CEI enforcement patterns, invariant frameworks) to decisions.md .