building-secure-contracts

Building Secure Contracts Skill

Safety Notice

This listing is imported from skills.sh public index metadata. Review upstream SKILL.md and repository scripts before running.

Copy this and send it to your AI assistant to learn

Install skill "building-secure-contracts" with this command: npx skills add oimiragieo/agent-studio/oimiragieo-agent-studio-building-secure-contracts

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
    • 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

InvariantStatusEvidence
sum(balances) == balanceVERIFIEDL90 invariant check
totalSupply == sum(shares)UNVERIFIEDNo 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-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 .

Source Transparency

This detail page is rendered from real SKILL.md content. Trust labels are metadata-based hints, not a safety guarantee.

Related Skills

Related by shared tags or category signals.

Automation

filesystem

No summary provided by upstream source.

Repository SourceNeeds Review
Automation

slack-notifications

No summary provided by upstream source.

Repository SourceNeeds Review
Automation

chrome-browser

No summary provided by upstream source.

Repository SourceNeeds Review
Automation

text-to-sql

No summary provided by upstream source.

Repository SourceNeeds Review