oimiragieo 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
```solidity
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
- `unchecked` block? → 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
1. **Solvency**: sum(balances) == address(this).balance — [VERIFIED L90]
2. **Total supply**: totalSupply == sum(all user shares) — [UNVERIFIED]
3. **Fee bound**: fee <= MAX_FEE (1000 bps) — [VERIFIED by require at L45]
4. **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
1. [Critical] Fix reentrancy in withdraw() before deployment
2. [High] Add reentrancy guard as defense-in-depth
3. [Medium] Add formal invariant tests via Foundry invariant suite
Integration with Agent-Studio
Recommended Workflow
- Invoke
audit-context-buildingfor initial code reconnaissance - Invoke
building-secure-contractsfor contract-specific analysis - Feed findings into
security-architectfor threat modeling - Use
static-analysis(Semgrep/CodeQL) for automated confirmation - Use
medusa-securityfor 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.