# audit-reentrancy

Audits Solidity smart contracts for reentrancy vulnerabilities including token transfer reentrancy via ERC777/callback tokens, state updates after external calls enabling draining, cross-function reentrancy manipulating shared state, and read-only reentrancy exploiting stale state during callbacks (project)

- **Kind:** skill
- **Source:** https://github.com/auditmos/skills
- **Page:** https://forefy.com/skills/88752292-53c9-45c4-8455-d037a6c44d55
- **API (JSON + files):** https://forefy.com/api/asr/88752292-53c9-45c4-8455-d037a6c44d55

---

## SKILL.md

---
name: audit-reentrancy
description: Audits Solidity smart contracts for reentrancy vulnerabilities including token transfer reentrancy via ERC777/callback tokens, state updates after external calls enabling draining, cross-function reentrancy manipulating shared state, and read-only reentrancy exploiting stale state during callbacks (project)
allowed-tools: Read, Grep, Glob
license: MIT
compatibility: Designed for Claude Code (or similar products)
metadata:
  author: Tomasz Kowalczyk (tom@auditmos.com)
  version: "1.0"
---

# Reentrancy Auditor

## When to Use
- Auditing external calls, token transfers, state management
- User mentions: reentrancy, ERC777, callback, CEI pattern, nonReentrant, external call, state update
- Analyzing call order, state synchronization, view functions
- Reviewing token transfer patterns, external integrations

## Audit Workflow

**IMPORTANT: Announce skill usage at the start of analysis**

Begin with: "I'm using the **audit-reentrancy** skill to analyze this contract for reentrancy vulnerabilities..."

1. **Scan for reentrancy vectors**
   - Search: `transfer`, `call`, `delegatecall`, `external`, `nonReentrant`, `ERC777`, `tokensReceived`, `onERC721Received`
   - Focus: external calls, token transfers, state updates, callback hooks

2. **Check against vulnerability patterns**
   - Reference `reference.md` for complete checklist
   - Compare code against `example.md`

3. **Validate exploitability**
   - **Check access control first** - grep for `onlyOwner|onlyAdmin|onlyGovernance` modifiers
   - Can non-privileged actors exploit the reentrancy?
   - Are state updates after external calls?
   - Can callbacks reenter different functions?
   - Do view functions read stale state during reentrancy?
   - Can attackers control callback timing?
   - Verify no compensating protections exist
   - Downgrade severity if admin-only unless cross-function attack exists

4. **Generate report**
   - Use deliverable template below
   - Include attack flow and PoC
   - Rank by severity

## Core Vulnerability Patterns

See `reference.md` for full checklist. Key patterns:

1. Token transfer reentrancy → ERC777/callback tokens allow reentrancy during transfers
2. State update after external call → transfer-before-update pattern enables draining
3. Cross-function reentrancy → reenter different functions to manipulate shared state
4. Read-only reentrancy → read stale state during reentrancy for profit

**Code examples:** See `example.md`

## Severity Criteria

**Critical:** State updates after external calls enabling direct fund draining, cross-function reentrancy manipulating critical shared state, **MUST be exploitable by non-privileged actors**
**High:** Token transfer reentrancy without nonReentrant protection, read-only reentrancy enabling price manipulation exploits, **MUST be exploitable by non-privileged actors**
**Medium:** Partial CEI violations with limited impact, missing nonReentrant on non-critical functions, admin-only CEI violations with cascading impact
**Low:** View function reentrancy without exploitable impact, theoretical reentrancy with no attack vector, admin-only CEI issues without immediate user impact

**IMPORTANT:** Admin-only functions (onlyOwner, onlyAdmin) with CEI violations are **LOW severity** unless:
- Admin function makes external calls that can reenter user-facing functions
- CEI violation enables governance attack to drain user funds
- Missing nonReentrant allows admin+user reentrancy combo attack

## False Positives - Do NOT Flag

- Internal functions (not externally callable)
- View functions reading non-critical state
- Contracts explicitly designed for trusted tokens only
- External calls with documented reentrancy safety analysis
- Functions with nonReentrant modifier properly applied

## Deliverable Format

**MANDATORY:** Before deliverable, verify each `checklist.md` item against codebase. Flag violations as findings.

Use template: `templates/report-template.md`

Each finding includes: severity, pattern #, file/lines, description, vulnerable code, attack flow, PoC showing fund draining, remediation.

## Key Principles

- **CEI pattern** - Checks, Effects, Interactions (state changes before external calls)
- **Mutex protection** - nonReentrant modifier on state-changing functions
- **Token awareness** - assume any token can have callbacks
- **Cross-function analysis** - consider reentering different functions
- **Read safety** - view functions must handle reentrancy

## Output Guidelines

**DO:**
- Reference specific lines and functions
- Provide complete attack flow with reentry point
- Show PoCs demonstrating fund draining
- Identify all shared state accessed
- Map cross-function reentrancy paths

**DON'T:**
- Report theoretical reentrancy without exploit path
- Flag internal functions (not exploitable)
- Ignore nonReentrant modifiers already in place
- Miss cross-function reentrancy (most common miss)

## checklist.md

# Reentrancy Security Checklist

Verify each item before finalizing audit report:

- [ ] **CEI pattern:** State changes before external calls (Checks-Effects-Interactions)
- [ ] **NonReentrant modifiers:** Applied to all state-changing functions with external calls
- [ ] **Token assumptions:** No assumptions about token transfer behavior (assume callbacks possible)
- [ ] **Cross-function analysis:** Shared state variables protected across all functions
- [ ] **Read-only safety:** View functions return consistent values during reentrancy or document limitations

## example.md

# Reentrancy Vulnerability Examples

## Pattern #1: Token Transfer Reentrancy

### VULNERABLE
```solidity
contract VulnerableERC777 {
    mapping(address => uint256) public balances;

    // ISSUE: ERC777 token can reenter during transfer
    function withdraw(uint256 amount) external {
        require(balances[msg.sender] >= amount, "Insufficient balance");

        // ERC777 tokensReceived callback triggered here
        // Attacker can reenter withdraw() before balance updated
        token.transfer(msg.sender, amount);

        // State updated AFTER external call - classic reentrancy
        balances[msg.sender] -= amount;
    }
}
```

### FIXED
```solidity
contract FixedERC777 {
    mapping(address => uint256) public balances;
    bool private locked;

    modifier nonReentrant() {
        require(!locked, "No reentrancy");
        locked = true;
        _;
        locked = false;
    }

    function withdraw(uint256 amount) external nonReentrant {
        require(balances[msg.sender] >= amount, "Insufficient balance");

        // CEI Pattern: Update state BEFORE external call
        balances[msg.sender] -= amount;

        // Safe: state already updated, reentrancy blocked
        token.transfer(msg.sender, amount);
    }
}
```

## Pattern #2: State Update After External Call

### VULNERABLE
```solidity
contract VulnerableWithdraw {
    mapping(address => uint256) public balances;

    function withdraw() external {
        uint256 balance = balances[msg.sender];
        require(balance > 0, "No balance");

        // ISSUE: ETH sent before state update
        // Attacker can reenter via receive() fallback
        (bool success, ) = msg.sender.call{value: balance}("");
        require(success, "Transfer failed");

        // State updated after external call
        balances[msg.sender] = 0;
    }

    // Attack contract:
    // receive() external payable {
    //     if (address(vulnerable).balance > 0) {
    //         vulnerable.withdraw(); // Reenter!
    //     }
    // }
}
```

### FIXED
```solidity
contract FixedWithdraw {
    mapping(address => uint256) public balances;

    function withdraw() external {
        uint256 balance = balances[msg.sender];
        require(balance > 0, "No balance");

        // CEI Pattern: Update state FIRST
        balances[msg.sender] = 0;

        // Safe: balance already zeroed, reentry has no effect
        (bool success, ) = msg.sender.call{value: balance}("");
        require(success, "Transfer failed");
    }
}
```

## Pattern #3: Cross-Function Reentrancy

### VULNERABLE
```solidity
contract VulnerableCrossFunctionReentrancy {
    mapping(address => uint256) public balances;
    mapping(address => uint256) public locked;

    // ISSUE: withdraw() and transfer() share balances state
    function withdraw(uint256 amount) external {
        require(balances[msg.sender] >= amount, "Insufficient");

        // External call BEFORE state update
        (bool success, ) = msg.sender.call{value: amount}("");
        require(success, "Failed");

        // Attacker reenters transfer() here
        balances[msg.sender] -= amount;
    }

    function transfer(address to, uint256 amount) external {
        // Uses stale balances[msg.sender] value
        require(balances[msg.sender] >= amount, "Insufficient");

        balances[msg.sender] -= amount;
        balances[to] += amount;
    }

    // Attack: withdraw() → reenter transfer() → drain funds
}
```

### FIXED
```solidity
contract FixedCrossFunctionReentrancy {
    mapping(address => uint256) public balances;
    bool private locked;

    modifier nonReentrant() {
        require(!locked, "No reentrancy");
        locked = true;
        _;
        locked = false;
    }

    // Both functions protected by same nonReentrant guard
    function withdraw(uint256 amount) external nonReentrant {
        require(balances[msg.sender] >= amount, "Insufficient");

        balances[msg.sender] -= amount; // CEI pattern

        (bool success, ) = msg.sender.call{value: amount}("");
        require(success, "Failed");
    }

    function transfer(address to, uint256 amount) external nonReentrant {
        require(balances[msg.sender] >= amount, "Insufficient");

        balances[msg.sender] -= amount;
        balances[to] += amount;
    }
}
```

## Pattern #4: Read-Only Reentrancy

### VULNERABLE
```solidity
contract VulnerablePool {
    uint256 public totalAssets;
    uint256 public totalShares;

    function withdraw(uint256 shares) external {
        uint256 assets = (shares * totalAssets) / totalShares;

        // ISSUE: totalAssets reduced before external call
        totalAssets -= assets;
        totalShares -= shares;

        // During callback, getPrice() returns inflated value
        token.transfer(msg.sender, assets);
    }

    // View function reads stale state during reentrancy
    function getPrice() public view returns (uint256) {
        // If called during withdraw callback:
        // totalAssets reduced but totalShares not yet updated
        // Returns incorrect price
        return (totalAssets * 1e18) / totalShares;
    }
}

contract ExternalProtocol {
    VulnerablePool pool;

    function liquidate(address user) external {
        // ISSUE: Reads price during reentrancy
        uint256 price = pool.getPrice(); // Stale/manipulated value

        // Makes decision based on wrong price
        uint256 collateralValue = userShares * price / 1e18;
        // ... liquidation logic
    }
}
```

### FIXED
```solidity
contract FixedPool {
    uint256 public totalAssets;
    uint256 public totalShares;
    bool private locked;

    modifier nonReentrant() {
        require(!locked, "No reentrancy");
        locked = true;
        _;
        locked = false;
    }

    function withdraw(uint256 shares) external nonReentrant {
        uint256 assets = (shares * totalAssets) / totalShares;

        totalAssets -= assets;
        totalShares -= shares;

        token.transfer(msg.sender, assets);
    }

    function getPrice() public view returns (uint256) {
        // If locked, price calculation may be inconsistent
        return (totalAssets * 1e18) / totalShares;
    }

    // Alternative: Add reentrancy check to view function
    function getPriceSafe() public view returns (uint256) {
        require(!locked, "Price inconsistent during operation");
        return (totalAssets * 1e18) / totalShares;
    }
}
```

## Advanced Example: Cross-Contract Reentrancy

### VULNERABLE
```solidity
contract VaultA {
    mapping(address => uint256) public deposits;

    function withdraw() external {
        uint256 amount = deposits[msg.sender];

        // Calls VaultB during withdrawal
        IVaultB(vaultB).notifyWithdrawal(msg.sender, amount);

        // State updated after external call
        deposits[msg.sender] = 0;
        token.transfer(msg.sender, amount);
    }
}

contract VaultB {
    function notifyWithdrawal(address user, uint256 amount) external {
        // Attacker reenters VaultA.withdraw() from here
        // VaultA.deposits[user] still non-zero
    }
}
```

### FIXED
```solidity
contract VaultA {
    mapping(address => uint256) public deposits;
    bool private locked;

    modifier nonReentrant() {
        require(!locked, "No reentrancy");
        locked = true;
        _;
        locked = false;
    }

    function withdraw() external nonReentrant {
        uint256 amount = deposits[msg.sender];

        // CEI: Update state FIRST
        deposits[msg.sender] = 0;

        // External calls after state changes
        IVaultB(vaultB).notifyWithdrawal(msg.sender, amount);
        token.transfer(msg.sender, amount);
    }
}
```

## Complex Example: ERC721 Callback Reentrancy

### VULNERABLE
```solidity
contract VulnerableNFTMarketplace {
    mapping(uint256 => address) public nftOwner;
    mapping(address => uint256) public balances;

    function buyNFT(uint256 tokenId) external payable {
        address seller = nftOwner[tokenId];
        uint256 price = getPrice(tokenId);

        require(msg.value >= price, "Insufficient payment");

        // ISSUE: NFT transfer triggers onERC721Received callback
        // Attacker can reenter before state updates
        nft.safeTransferFrom(address(this), msg.sender, tokenId);

        // State updated AFTER external call
        nftOwner[tokenId] = msg.sender;
        balances[seller] += price;

        // Attacker can call buyNFT again with same tokenId
        // before ownership recorded
    }
}
```

### FIXED
```solidity
contract FixedNFTMarketplace {
    mapping(uint256 => address) public nftOwner;
    mapping(address => uint256) public balances;

    function buyNFT(uint256 tokenId) external payable nonReentrant {
        address seller = nftOwner[tokenId];
        uint256 price = getPrice(tokenId);

        require(msg.value >= price, "Insufficient payment");

        // CEI: Update state FIRST
        nftOwner[tokenId] = msg.sender;
        balances[seller] += price;

        // Safe: state already updated
        nft.safeTransferFrom(address(this), msg.sender, tokenId);
    }
}
```

## Summary: Key Protections

1. **CEI Pattern:** Checks → Effects (state changes) → Interactions (external calls)
2. **NonReentrant modifier:** Use OpenZeppelin's ReentrancyGuard on all state-changing functions
3. **Token awareness:** Assume all tokens can have callbacks (ERC777, ERC721, malicious ERC20)
4. **Cross-function protection:** Apply nonReentrant to all functions sharing state
5. **Read-only safety:** Document view function limitations during reentrancy or add guards

## Attack Flow Template

```
1. User calls vulnerable function
2. Function performs checks
3. Function makes external call (transfer, call, etc.)
4. Attacker's callback triggered
5. Attacker reenters same or different function
6. Shared state still in old value
7. Exploit stale state to drain funds
8. Original function continues, updates state too late
```

## Detection Checklist

For each function with external calls:
- [ ] Is state updated before the external call?
- [ ] Is nonReentrant modifier present?
- [ ] Can other functions be reentered that share state?
- [ ] Do view functions return stale values during callback?
- [ ] Are token callbacks (ERC777, ERC721) considered?

## reference.md

# Reentrancy Vulnerability Patterns

## Pattern #1: Token Transfer Reentrancy
**Risk:** ERC777, ERC721, or tokens with callback hooks allow reentrancy during token transfers
**Detection:** Check if token transfers occur before state updates, or if nonReentrant modifier is missing
**Impact:** Attacker can reenter during token transfer callback to manipulate state or drain funds

## Pattern #2: State Update After External Call
**Risk:** State variables updated after external calls (transfer, call, etc.) violate CEI pattern
**Detection:** Verify all state changes occur before external calls in function execution flow
**Impact:** Attacker reenters between external call and state update to drain funds via repeated withdrawals

## Pattern #3: Cross-Function Reentrancy
**Risk:** Function A makes external call, attacker reenters Function B to manipulate shared state
**Detection:** Map all external calls and check if other functions can be called that access same state variables
**Impact:** Bypass single-function reentrancy guards by reentering different function that shares state

## Pattern #4: Read-Only Reentrancy
**Risk:** View/pure functions read state during reentrancy callback, return stale values used for critical decisions
**Detection:** Check if external protocols rely on view functions that could return inconsistent state during reentrancy
**Impact:** Attacker exploits stale state reads to manipulate prices, collateral ratios, or other derived values

## templates

```

```

## templates/report-template.md

# Reentrancy Security Audit Report

## Executive Summary

**Contract:** [Contract Name]
**Audit Date:** [Date]
**Auditor:** [Name/Team]

**Findings Overview:**
- Critical: X
- High: X
- Medium: X
- Low: X

## Findings

---

### [SEVERITY] Finding #X: [Vulnerability Title]

**Pattern:** #X - [Pattern Name from reference.md]

**Location:** `[contract_name.sol:line_numbers]`

**Description:**

[Detailed explanation of the reentrancy vulnerability]

**Vulnerable Code:**

```solidity
function vulnerableFunction() external {
    // Highlight the problematic call order
    externalCall(); // External call
    stateVariable = newValue; // State update after call - ISSUE
}
```

**Attack Flow:**

1. [Attacker calls vulnerable function]
2. [Function performs checks]
3. [Function makes external call at line X]
4. [Attacker's callback triggered (fallback/receive/tokensReceived/onERC721Received)]
5. [Attacker reenters function Y]
6. [Shared state variable Z still has old value]
7. [Attacker exploits stale state to extract funds]
8. [Original function continues and updates state]
9. [Result: funds drained/double-spent]

**Proof of Concept:**

```solidity
contract ReentrancyAttack {
    VulnerableContract target;
    uint256 attackCount;

    function attack() external payable {
        target.deposit{value: 1 ether}();
        target.withdraw(1 ether);
    }

    // Callback triggered during withdraw
    receive() external payable {
        if (attackCount < 10 && address(target).balance > 0) {
            attackCount++;
            target.withdraw(1 ether); // Reenter
        }
    }
}
```

**Test Case:**

```solidity
function testReentrancyExploit() public {
    // Setup
    target.deposit{value: 10 ether}(attacker);

    // Execute attack
    vm.prank(attacker);
    attackContract.attack{value: 1 ether}();

    // Verify: attacker withdrew more than deposited
    assertGt(attacker.balance, 1 ether);
    // Expected: 1 ether, Actual: 10+ ether
}
```

**Impact Analysis:**

**Direct Impact:**
- [Fund draining - quantify amount]
- [State corruption - describe affected variables]

**Systemic Impact:**
- [Protocol insolvency if widely exploited]
- [User fund loss]

**Affected Functions:**
- [List all functions sharing vulnerable state]

**Remediation:**

```solidity
// Option 1: CEI Pattern
function fixedWithCEI() external {
    // Checks
    require(balances[msg.sender] >= amount, "Insufficient");

    // Effects (state changes FIRST)
    balances[msg.sender] -= amount;

    // Interactions (external calls LAST)
    token.transfer(msg.sender, amount);
}

// Option 2: Reentrancy Guard
function fixedWithGuard() external nonReentrant {
    require(balances[msg.sender] >= amount, "Insufficient");

    token.transfer(msg.sender, amount);
    balances[msg.sender] -= amount;
}

// Option 3: Both (recommended)
function fixedWithBoth() external nonReentrant {
    require(balances[msg.sender] >= amount, "Insufficient");

    balances[msg.sender] -= amount; // CEI
    token.transfer(msg.sender, amount);
}
```

**Recommendations:**
1. [Primary fix - e.g., "Apply nonReentrant modifier to all state-changing functions"]
2. [Secondary fix - e.g., "Reorder operations to follow CEI pattern"]
3. [Defense in depth - e.g., "Add both nonReentrant and CEI pattern"]

**Gas Impact:** [Estimated additional gas cost for fix]
- nonReentrant: ~2,100 gas per call
- State reordering: 0 gas (optimization)

---

### [SEVERITY] Finding #X: [Next Vulnerability]

[Repeat above structure for each finding]

---

## Severity Definitions

**Critical:** State updates after external calls enabling direct fund draining, cross-function reentrancy manipulating critical shared state without guards.

**High:** Token transfer reentrancy without nonReentrant protection on functions managing funds, read-only reentrancy enabling price manipulation for liquidations.

**Medium:** Partial CEI violations with limited impact, missing nonReentrant on non-critical state-changing functions, cross-function reentrancy on low-value operations.

**Low:** View function reentrancy without exploitable impact, theoretical reentrancy with no viable attack vector, reentrancy on internal functions.

## Recommendations Summary

### Immediate Actions (Critical/High)
1. [List critical fixes]
   - Example: "Apply nonReentrant to withdraw(), transfer(), and deposit()"
   - Example: "Reorder state updates before external calls in withdraw()"

### Short-term Improvements (Medium)
1. [List medium-priority enhancements]
   - Example: "Add nonReentrant to remaining state-changing functions"
   - Example: "Document view function limitations during reentrancy"

### Long-term Enhancements (Low)
1. [List optimization opportunities]
   - Example: "Audit all external integrations for callback patterns"

## Checklist Results

Based on `checklist.md`:

- [x] **CEI pattern:** State changes before external calls ✓/✗
- [x] **NonReentrant modifiers:** Applied to state-changing functions ✓/✗
- [x] **Token assumptions:** No assumptions about token behavior ✓/✗
- [x] **Cross-function analysis:** Shared state protected across functions ✓/✗
- [x] **Read-only safety:** View functions handle reentrancy safely ✓/✗

## Function Analysis

### External Call Inventory

| Function | External Call | State Updates After? | NonReentrant? | Risk |
|----------|---------------|---------------------|---------------|------|
| withdraw() | token.transfer() | Yes | No | CRITICAL |
| deposit() | token.transferFrom() | No | No | Low |
| swap() | pool.swap() | Yes | Yes | Medium |

### State Variable Access Map

| State Variable | Functions Accessing | Protected? |
|----------------|-------------------|------------|
| balances | withdraw, transfer, deposit | No |
| totalSupply | mint, burn | Yes |

### Cross-Function Reentrancy Paths

```
withdraw() → external call → attacker callback
    ↓
    reenter transfer() (same balances state)
    ↓
    exploit stale balances[msg.sender]
```

## Testing Recommendations

### Unit Tests
- [ ] Reentrancy attack on withdraw()
- [ ] Cross-function reentrancy via transfer()
- [ ] ERC777 tokensReceived callback test
- [ ] ERC721 onERC721Received callback test
- [ ] Read-only reentrancy on view functions

### Integration Tests
- [ ] Multi-step reentrancy attack
- [ ] Cross-contract reentrancy scenarios
- [ ] Callback from external protocols

### Fuzz Tests
- [ ] Random reentry points during execution
- [ ] State consistency during callbacks

## Appendix

### CEI Pattern Explanation

**Checks-Effects-Interactions Pattern:**

```solidity
function followsCEI() external {
    // 1. CHECKS - Validate inputs and conditions
    require(balances[msg.sender] >= amount, "Insufficient");
    require(amount > 0, "Zero amount");

    // 2. EFFECTS - Update state variables
    balances[msg.sender] -= amount;
    totalSupply -= amount;
    emit Withdrawal(msg.sender, amount);

    // 3. INTERACTIONS - External calls LAST
    token.transfer(msg.sender, amount);
    externalContract.notify(msg.sender, amount);
}
```

### Common Reentrancy Vectors

1. **ETH transfers:** `call`, `transfer`, `send`
2. **Token callbacks:** ERC777 `tokensReceived`, ERC721 `onERC721Received`
3. **External calls:** `call`, `delegatecall`, `staticcall`
4. **Interface calls:** Any external contract interaction

### OpenZeppelin ReentrancyGuard

```solidity
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract MyContract is ReentrancyGuard {
    function protectedFunction() external nonReentrant {
        // Function logic
    }
}
```

### Cross-Function Reentrancy Example

```
Contract state: balance = 100

1. User calls withdraw(50)
2. withdraw() sends 50 ETH (balance still 100 in state)
3. User's receive() callback reenters transfer(50, attacker)
4. transfer() checks balance (still 100), allows transfer
5. Both operations succeed, user extracted 100 ETH
```