Smart Contract Security Auditing: Complete Guide

// VULNERABLE CONTRACT
contract VulnerableBank {
    mapping(address => uint256) public balances;
    
    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }
    
    // VULNERABLE: External call before state update
    function withdraw() external {
        uint256 balance = balances[msg.sender];
        require(balance > 0, "No balance");
        
        // External call BEFORE state update - VULNERABLE
        (bool success, ) = msg.sender.call{value: balance}("");
        require(success, "Transfer failed");
        
        // State updated AFTER external call - TOO LATE!
        balances[msg.sender] = 0;
    }
}

// ATTACK CONTRACT
contract Attacker {
    VulnerableBank public bank;
    address public owner;
    
    receive() external payable {
        if (address(bank).balance >= 1 ether) {
            bank.withdraw();
        }
    }
    
    function attack() external payable {
        bank.deposit{value: 1 ether}();
        bank.withdraw();
        payable(owner).transfer(address(this).balance);
    }
}

// SECURE CONTRACT
contract SecureBank {
    mapping(address => uint256) public balances;
    
    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }
    
    function withdraw() external {
        uint256 balance = balances[msg.sender];
        require(balance > 0, "No balance");
        
        // 1. CHECK: Validate conditions
        require(balance > 0, "No balance");
        
        // 2. EFFECTS: Update state BEFORE external call
        balances[msg.sender] = 0;
        
        // 3. INTERACTIONS: Make external call AFTER state update
        (bool success, ) = msg.sender.call{value: balance}("");
        require(success, "Transfer failed");
    }
}

// Alternative: Use ReentrancyGuard
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract ModernBank is ReentrancyGuard {
    function withdraw() external nonReentrant {
        // Function is automatically protected
    }
}

// VULNERABLE: Solidity < 0.8 without SafeMath
function transfer(address to, uint256 amount) external {
    require(balances[msg.sender] - amount >= 0); // Won't work!
    balances[msg.sender] -= amount; // Can underflow!
    balances[to] += amount; // Can overflow!
}

// SECURE: Solidity 0.8+ (built-in overflow checks)
function transfer(address to, uint256 amount) external {
    balances[msg.sender] -= amount; // Automatically checked!
    balances[to] += amount; // Automatically checked!
}

// SECURE: Using OpenZeppelin's SafeMath (older Solidity)
import "@openzeppelin/contracts/utils/math/SafeMath.sol";

contract SafeToken {
    using SafeMath for uint256;
    
    function transfer(address to, uint256 amount) external {
        balances[msg.sender] = balances[msg.sender].sub(amount);
        balances[to] = balances[to].add(amount);
    }
}

// VULNERABLE: Missing access control
function setOwner(address newOwner) external {
    owner = newOwner; // Anyone can call!
}

// VULNERABLE: Public initialization
function initialize(address _owner) external {
    owner = _owner; // Can be front-run!
// Solution: Use initializer modifier
abstract contract Initializable {
    bool private _initialized;
    bool private _initializing;
    
    modifier initializer() {
        require(!_initialized && !_initializing);
        _initializing = true;
        _;
        _initialized = true;
        _initializing = false;
    }
}

// SECURE: Proper access control with OpenZeppelin
import "@openzeppelin/contracts/access/AccessControl.sol";

contract SecureContract is AccessControl {
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    
    function mint(address to, uint256 amount) external onlyRole(MINTER_ROLE) {
        _mint(to, amount);
    }
    
    function grantMinterRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
        grantRole(MINTER_ROLE, account);
    }
}

# Comprehensive Smart Contract Audit Checklist
security_checks:
  access_control:
    - "All critical functions have access controls"
    - "Role management is properly implemented"
    - "Ownable/AccessControl from OpenZeppelin used correctly"
    - "Initializer pattern used for proxy contracts"
    - "Ownership transfer has timelock or confirmation"
  
  arithmetic:
    - "Solidity 0.8+ used OR SafeMath for all math"
    - "All arithmetic operations checked for overflow"
    - "Precision loss considered in calculations"
    - "Rounding errors handled correctly"
  
  reentrancy:
    - "Checks-Effects-Interactions pattern followed"
    - "ReentrancyGuard used where applicable"
    - "No external calls before state changes"
    - "Callback functions reviewed carefully"
  
  oracle_manipulation:
    - "Oracle sources are decentralized"
    - "TWAP used for price feeds"
    - "Oracle data has adequate staleness checks"
    - "Multiple oracle sources for critical data"
  
  front_running:
    - "Commit-reveal schemes for sensitive operations"
    - "Batch auctions for fair price discovery"
    - "Private transactions where applicable"
  
  logic_errors:
    - "Edge cases handled (zero, max values)"
    - "State transitions are valid"
    - "Permissions cannot be bypassed"
    - "Access control cannot be circumvented"
  
  upgradeability:
    - "Proxy pattern properly implemented"
    - "Storage layout is compatible"
    - "Initializers cannot be re-run"
    - "Upgrade mechanism has proper access"

# Slither - Static Analysis Framework
pip install slither-analyzer
slither . --exclude-dependencies

# Sample Slither Output
$ slither contracts/Token.sol

INFO:Slither:.:
Token.sol:
  Variable: owner (visibility: internal)
  Variable: balances (mapping)
  Function: transfer (external)
  Function: mint (external)
  Reentrancy in transfer (medium):
    Reentrancy detected in transfer
    Call to: balances[msg.sender]
  
  Critical: Unprotected mint function
  High: Centralization risk

# Echidna Fuzzing Configuration
# echidna.config.yaml
testMode: "property"
coverage: true
corpusDir: "corpus"
testLimit: 50000

# Property-based testing
contract TestToken is ERC20 {
    function echidna_sender_balance() public view returns(bool) {
        return balances[msg.sender] <= totalSupply();
    }
    
    function echidna_mint_not_overflow() public view returns(bool) {
        return totalSupply() <= 1e10 * 10**18;
    }
}

# Run Echidna
echidna-test .

# Smart Contract Security Audit Report

## Executive Summary
- Project: [Protocol Name]
- Version: [Version audited]
- Date: [Audit Date]
- Auditors: [Audit Team]
- Overall Risk: [Critical/High/Medium/Low]

## Scope
- Contracts audited: [List]
- Deployed at: [Addresses]
- Gas costs: [Summary]

## Findings Summary
| Severity | Count | Resolved | Pending |
|----------|-------|----------|---------|
| Critical | X | X | 0 |
| High | X | X | X |
| Medium | X | X | X |
| Low | X | X | X |
| Informational | X | X | X |

## Detailed Findings

### [CRITICAL-01] Reentrancy Vulnerability
**Location**: `Pool.sol:142`
**Description**: The withdraw function has a reentrancy vulnerability...
**Impact**: Attacker can drain all funds from the protocol
**Recommendation**: Implement Checks-Effects-Interactions pattern...
**Status**: Fixed in v1.1

### [HIGH-01] Missing Access Control
**Location**: `Admin.sol:89`
**Description**: The pause function lacks access control...
**Impact**: Anyone can pause the protocol
**Recommendation**: Add onlyOwner modifier...
**Status**: Fixed

## Additional Recommendations
1. Upgrade to Solidity 0.8.x for built-in overflow checks
2. Implement timelock for critical admin functions
3. Add merkle proofs for airdrop claims
4. Consider formal verification for critical paths

## Conclusion
The audited contracts have [X] findings, [X] of which have been resolved.
After fixes, the contracts are ready for mainnet deployment.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/security/AccessControl.sol";
import "@openzeppelin/contracts/utils/Pausable.sol";

/**
 * @title SecureVault
 * @dev A secure vault implementation following best practices
 */
contract SecureVault is ReentrancyGuard, AccessControl, Pausable {
    bytes32 public constant GUARDIAN_ROLE = keccak256("GUARDIAN_ROLE");
    
    mapping(address => uint256) private balances;
    uint256 public totalDeposits;
    
    // Events for transparency
    event Deposit(address indexed user, uint256 amount);
    event Withdrawal(address indexed user, uint256 amount);
    event EmergencyWithdraw(address indexed user, uint256 amount);
    
    constructor() {
        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _grantRole(GUARDIAN_ROLE, msg.sender);
    }
    
    /**
     * @dev Deposit funds into the vault
     * Following Checks-Effects-Interactions pattern
     */
    function deposit() external payable whenNotPaused {
        // 1. Checks
        require(msg.value > 0, "Cannot deposit 0");
        
        // 2. Effects (state updates before external calls)
        balances[msg.sender] += msg.value;
        totalDeposits += msg.value;
        
        // 3. Interactions (external calls last)
        emit Deposit(msg.sender, msg.value);
    }
    
    /**
     * @dev Withdraw funds from the vault
     */
    function withdraw(uint256 amount) external nonReentrant whenNotPaused {
        // 1. Checks
        require(amount > 0, "Cannot withdraw 0");
        require(balances[msg.sender] >= amount, "Insufficient balance");
        
        // 2. Effects
        balances[msg.sender] -= amount;
        totalDeposits -= amount;
        
        // 3. Interactions
        (bool success, ) = msg.sender.call{value: amount}("");
        require(success, "Transfer failed");
        
        emit Withdrawal(msg.sender, amount);
    }
    
    /**
     * @dev Emergency pause - only guardians can trigger
     */
    function pause() external onlyRole(GUARDIAN_ROLE) {
        _pause();
    }
    
    /**
     * @dev Emergency withdrawal for admins
     */
    function emergencyWithdraw() external onlyRole(DEFAULT_ADMIN_ROLE) {
        uint256 balance = address(this).balance;
        (bool success, ) = msg.sender.call{value: balance}("");
        require(success);
        emit EmergencyWithdraw(msg.sender, balance);
    }
    
    // Receive function to accept ETH
    receive() external payable {
        deposit();
    }
}

(* Simple Formal Verification Example in Coq *)
(* Proving that addition is commutative *)

Lemma add_comm : forall n m : nat, n + m = m + n.
Proof.
  intros n m.
  induction n as [| n' IHn'].
  - simpl. rewrite <- plus_n_O. reflexivity.
  - simpl. rewrite IHn'. rewrite plus_n_Sm. reflexivity.
Qed.

(* This ensures the mathematical property holds for all inputs *)

# Sample Bug Bounty Program Configuration
bug_bounty:
  platform: "Immunefi"
  rewards:
    critical: 
      min: 50000
      max: 500000
      criteria: "Funds at risk"
    high:
      min: 10000
      max: 50000
      criteria: "Significant functionality impact"
    medium:
      min: 2500
      max: 10000
      criteria: "Moderate impact"
    low:
      min: 500
      max: 2500
      criteria: "Minor issues"
  
  scope:
    contracts:
      - "contracts/Vault.sol"
      - "contracts/Token.sol"
    excluded:
      - "Test contracts"
      - "Deprecated functions"
  
  requirements:
    - "PoC required for critical/high"
    - "No DOS attacks on testnets"
    - "Disclosure before public"
    - "Timeline: 7 days for triage"