Reentrancy Remediation

Overview

Related Detector: Reentrancy

Reentrancy occurs when a contract makes an external call before completing its own state updates, allowing the callee to re-enter and exploit stale state. The core remediation is the Checks-Effects-Interactions (CEI) pattern: always complete all state modifications before executing any external call.

Recommended Fix

Before (Vulnerable)

// Anti-pattern: Interactions before Effects
contract VulnerableVault {
    mapping(address => uint256) public balances;

    function withdraw(uint256 amount) external {
        require(balances[msg.sender] >= amount, "Insufficient balance");
        // INTERACTION before EFFECT — reentrancy window is open here
        (bool success, ) = msg.sender.call{value: amount}("");
        require(success, "Transfer failed");
        balances[msg.sender] -= amount; // Too late — attacker already re-entered
    }
}

After (Fixed — Checks-Effects-Interactions)

// Correct: Effects before Interactions
contract SafeVault {
    mapping(address => uint256) public balances;

    function withdraw(uint256 amount) external {
        // CHECK
        require(balances[msg.sender] >= amount, "Insufficient balance");
        // EFFECT (state update) — must happen before any external call
        balances[msg.sender] -= amount;
        // INTERACTION (external call) — safe because state is already updated
        (bool success, ) = msg.sender.call{value: amount}("");
        require(success, "Transfer failed");
    }
}

The CEI pattern ensures that even if a malicious callee re-enters withdraw(), the balance has already been set to zero, causing the re-entrant call to fail the require check.

Alternative Mitigations

1. Reentrancy Guard (Mutex)

Use OpenZeppelin’s ReentrancyGuard or implement a mutex manually:

import "@audited/utils/ReentrancyGuard.sol";

contract GuardedVault is ReentrancyGuard {
    mapping(address => uint256) public balances;

    function withdraw(uint256 amount) external nonReentrant {
        require(balances[msg.sender] >= amount, "Insufficient balance");
        balances[msg.sender] -= amount;
        (bool success, ) = msg.sender.call{value: amount}("");
        require(success, "Transfer failed");
    }
}

The nonReentrant modifier sets a flag before execution and clears it after — any reentrant call sees the flag and reverts. This is useful when the CEI pattern is difficult to apply (e.g., complex multi-step operations).

2. Pull-Payment Pattern

Instead of pushing ETH to users, let them pull it:

contract PullPaymentVault {
    mapping(address => uint256) public pendingWithdrawals;

    // No external call in the business logic function
    function closePosition(address user, uint256 amount) internal {
        pendingWithdrawals[user] += amount;
    }

    // User initiates withdrawal — reentrancy risk is contained here
    function withdraw() external {
        uint256 amount = pendingWithdrawals[msg.sender];
        require(amount > 0, "Nothing to withdraw");
        pendingWithdrawals[msg.sender] = 0; // Effect before interaction
        payable(msg.sender).transfer(amount);
    }
}

Common Mistakes

Mistake 1: Applying the fix in the wrong order

// STILL VULNERABLE — the balance reduction happens in a nested function
// called AFTER the external call
function withdraw(uint256 amount) external {
    require(balances[msg.sender] >= amount);
    (bool success, ) = msg.sender.call{value: amount}("");
    require(success);
    _updateBalance(msg.sender, amount); // Internal call still happens after external call
}

function _updateBalance(address user, uint256 amount) internal {
    balances[user] -= amount;
}

Mistake 2: Assuming `transfer()` is safe from reentrancy

// OUTDATED: transfer() and send() are limited to 2300 gas stipend in older EVM
// but this was not changed in newer EVMs and the pattern is now discouraged
address.transfer(amount); // Not reliably safe — use .call{} with CEI instead

Mistake 3: Cross-function reentrancy through shared state

// VULNERABLE: withdraw() and borrowAgainst() share balances mapping
// An attacker can re-enter borrowAgainst() from withdraw()'s external call
// before withdraw() updates balances
function withdraw(uint256 amount) external {
    (bool success, ) = msg.sender.call{value: amount}(""); // Re-enters borrowAgainst()
    balances[msg.sender] -= amount;
}

function borrowAgainst(uint256 collateral) external {
    // Sees stale (pre-withdrawal) balance as collateral
    require(balances[msg.sender] >= collateral);
    _issueLoan(msg.sender, collateral);
}

Use cross-function-reentrancy detector to find these patterns and apply the nonReentrant guard across all related functions.