Stop Using Solidity’s transfer() Now

This article was originally published at https://diligence.consensys.net. Please read it there, where it includes code examples and has better formatting.

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 hardcoded amount of gas a contract’s fallback function receives if it’s called via Solidity’s transfer() or send() methods.

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.

Gas Costs Can and Will Change

Each opcode supported by the EVM has an associated gas cost. For example, SLOAD, which reads a word from storage, currently-but not for long-costs 200 gas. The gas costs aren't arbitrary. They're meant to reflect the underlying resources consumed by each operation on the nodes that make up Ethereum.

From the EIP’s motivation section:

An imbalance between the price of an operation and the resource consumption (CPU time, memory etc) has several drawbacks:

* It could be used for attacks, by filling blocks with underpriced operations which causes excessive block processing time.

* Underpriced opcodes cause a skewed block gas limit, where sometimes blocks finish quickly but other blocks with similar gas use finish slowly.

If operations are well-balanced, we can maximise the block gaslimit and have a more stable processing time.

SLOAD has historically been underpriced, and EIP 1884 rectifies that.

Smart Contracts Can’t Depend on Gas Costs

If gas costs are subject to change, then smart contracts can’t depend on any particular gas costs.

Any smart contract that uses transfer() or send() is taking a hard dependency on gas costs by forwarding a fixed amount of gas: 2300.

Our recommendation is to stop using transfer() and send() in your code and switch to using call() instead:

Please see the original post for code!

Other than the amount of gas forwarded, these two contracts are equivalent.

What About Reentrancy?

This was hopefully your first thought upon seeing the above code. The whole reason transfer() and send() were introduced was to address the cause of the infamous hack on The DAO. The idea was that 2300 gas is enough to emit a log entry but insufficient to make a reentrant call that then modifies storage.

Remember, though, that gas costs are subject to change, which means this is a bad way to address reentrancy anyway. Earlier this year, the Constantinople fork was delayed because lowering gas costs caused code that was previously safe from reentrancy to no longer be.

If we’re not going to use transfer() and send() anymore, we'll have to protect against reentrancy in more robust ways. Fortunately, there are good solutions for this problem.

Checks-Effects-Interactions Pattern

The simplest way to eliminate reentrancy bugs is to use the checks-effects-interactions pattern. Here’s a classic example of a reentrancy bug:

Please see the original post for code!

If msg.sender is a smart contract, it has an opportunity on line 6 to call withdraw() again before line 7 happens. In that second call, balanceOf[msg.sender] is still the original amount, so it will be transferred again. This can be repeated as many times as necessary to drain the smart contract.

The idea of the checks-effects-interactions pattern is to make sure that all your interactions (external calls) happen at the end. A typical fix for the above code is as follows:

Please see the original post for code!

Notice that in this code, the balance is zeroed out before the transfer, so attempting to make a reentrant call to withdraw() will not benefit an attacker.

Use a Reentrancy Guard

Another approach to preventing reentrancy is to explicitly check for and reject such calls. Here’s a simple version of a reentrancy guard so you can see the idea:

Please see the original post for code!

With this code, if a reentrant call is attempted, the require on line 7 will reject it because lock is still set to true.

A more sophisticated and gas-efficient version of this can be found in OpenZeppelin’s ReentrancyGuard contract. If you inherit from ReentrancyGuard, you just need to decorate functions with nonReentrant to prevent reentrancy.

Please note that this method only protects you if you explicitly apply it to all the right functions. It also carries an increased gas cost due to the need to persist a value in storage.

What About Vyper?

Vyper’s send() function uses the same hardcoded gas stipend as Solidity's transfer(), so it too is to be avoided. You can use instead.

Vyper has a @nonreentrant(<unique_key>) decorator built in that works similarly to OpenZeppelin's ReentrancyGuard.

Summary

• Recommending transfer() made sense under the assumption that gas costs are constant.
• Gas costs are not constant. Smart contracts should be robust to this fact.
• Solidity’s transfer() and send() use a hardcoded gas amount.
• These methods should be avoided. Use .call.value(...)("") instead.
• This carries a risk regarding reentrancy. Be sure to use one of the robust methods available for preventing reentrancy vulnerabilities.
• Vyper’s send() has the same problem.

Originally published at https://diligence.consensys.net on September 2, 2019.