Reentrancy in Solidity, and why withdraw() is where it lives

Reentrancy is the vulnerability that drained The DAO in 2016, and it is still one of the most common findings in Solidity audits today. The mechanism has not changed and neither has the fix. What follows is a complete, working example — the same one Weakspot is regression-tested against.

The vulnerable contract

This is a minimal ETH vault. Users deposit, then withdraw their balance. It looks correct, and it compiles without a single warning.

contract EtherVault {
    mapping(address => uint256) public balances;

    function deposit() external payable {
        balances[msg.sender] += msg.value;
    }

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

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

        balances[msg.sender] = 0;   // <-- too late
    }
}

Why it fails

The bug is one line of ordering. msg.sender.call hands control to the receiving address before balances[msg.sender] is zeroed. If msg.sender is a contract, that call runs its receive() or fallback() function — arbitrary code, executing while the vault still believes the caller has a full balance.

A contract that calls withdraw() again from inside its own receive() passes the require(amount > 0) check a second time, because nothing has been written to storage yet. The sequence:

• —Attacker deposits 1 ETH. Vault holds 100 ETH total; attacker's recorded balance is 1 ETH.
• —Attacker calls withdraw(). Vault reads amount = 1 ETH and sends it.
• —The send triggers the attacker's receive(), which calls withdraw() again — before line 3 of the original call has run.
• —Balance is still 1 ETH, so the check passes and another 1 ETH goes out. Repeat until the vault is empty.
• —The stack unwinds and balances[msg.sender] = 0 finally executes — once, against an already-drained contract.

The fix

Follow checks-effects-interactions: validate inputs, then write state, then make external calls — in that order, always. Moving one line fixes this contract completely.

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

    balances[msg.sender] = 0;   // effects BEFORE interactions

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

A re-entrant call now reads a zero balance and reverts on the require. As a second layer, OpenZeppelin's ReentrancyGuard and its nonReentrant modifier block the re-entry outright. Use both: the guard is insurance, correct ordering is the actual fix, and a guard on top of wrong ordering still leaves you exposed to cross-function reentrancy through a different entry point.