Missing return value bug — At least 130 tokens affected

Published in

Coinmonks

6 min readJun 5, 2018

tl;dr There is a critical bug in a lot of ERC20 token contracts that surfaced due to a recent Solidity update.

ERC Basics

The ERC20 standard is by far the most common token standard on the Ethereum platform. ERC20 is defined as an interface that specifies which functions and events have to be implemented in a smart contract to be ERC20 compliant. Currently, these are following:

What should the transfer function of an ERC20 contract return?

During the development of ERC20, there were lengthy discussions about the correct return values of the transfer function of an ERC20 contract. There were basically two camps in this discussion. One side argued that the transfer function should return false if the transfer failed to allow error handling in the calling contract. The other side claimed that an ERC20 should revert the transaction in case of failure to ensure safety. This matter was never really settled and both behaviors are now considered to be ERC20 compliant. The most common behavior is a revert in case of failure, especially since OpenZeppelin implemented their ERC20 token by this means.

All this has little to do with the actual bug I want to describe, but may be helpful to provide some context.

The Bug

It turned out that a significant percentage of ERC20 token behave in yet another way regarding the return values of the transfer function.

The transfer functions of these token contracts (let´s call them BadTokens) do not return anything. The interface they are complying to, looks like this:

This interface is not complying to the ERC20 standard, which means that the contracts implementing this interface are not ERC20 tokens.

One important point of having a token standard, is to enable other smart contracts to interact with many different token contracts through a common interface. There are many different contracts that use this feature of an ERC20 token. Decentralized exchanges can send tokens, crowdsale contracts can accept ERC20 token as payment and so on. Now the interesting question is, what happens if a contract that expects an ERC20 interface is trying to interact with a BadToken that is not ERC20 compliant?

Let´s have a look at this scenario:

In Solidity a function selector is derived from its function name and the type of the input parameters.

selector = bytes4(sha3(“transfer()”))

The return value of a function is not part of a function selector. So a function transfer() without a return value and a function transfer() returns(bool) have the same function selector but they are still different. If you would try

contract BadToken is Token {                      
   function transfer() {}         
}

the compiler would not accept the transfer() function as an implementation of the Token interface, because of the missing return value. So the GoodToken is an implementation of the Token interface, the BadToken is not.

What happens if we call it anyway?

The calling contract sends an external call to the BadToken, which processes the call, makes the transfer and does not return a Boolean return value. The calling contract now looks up the return value in the memory, but since the token did not write a return value it will take whatever it finds in this memory position as the return value of the external call.

This is already pretty bad: taking some data that happens to be in a memory slot as a return value is not a good idea.

By sheer coincidence this problem didn’t surface in the past because the memory slot in which the caller expected the return value, overlapped with the memory slot, the function selector of the call was stored in. This was interpreted by the EVM as the return value “true”. And so, by pure luck, the EVM behaved like the programmers intended it to behave.

But this is not the case for newer smart contracts.

What changed?

Since the Byzantium hard fork last October, the EVM has a new opcode, called RETURNDATASIZE. This opcode stores (as the name implies) the size of the returned data of an external call. This is a very useful opcode, since it allows to return dynamically sized arrays in function calls.

This opcode was adopted in the Solidity 0.4.22 update (when set to post-Byzantium mode, which is the default). Now, the code checks the size of the return value after an external call and reverts the transaction in case the return data is shorter than expected. This is a much safer behavior than reading data from some memory slot. But this new behavior is a huge problem for our BadTokens.

Who is affected.

To get an idea of how widespread this issue is, I took the list of ERC20 tokens listed on Etherdelta and checked their API verified on Etherscan. Thereby, I found 130 affected token contracts.

There are some big names on this list:

The biggest tokens (by Market Cap) on the list are:

Binance Coin $1.587.146.847
OmiseGO $1.127.641.627

But there are a lot of small and mid-sized tokens on this list, too.

What´s the risk?

As described above, the biggest risk is that a smart contract that is compiled with solc ≥ 0.4.22 , which is expecting an ERC20 interface, will not be able to interact with our BadTokens. This could mean that tokens which are send to such a contract, will be stuck there forever even if the contract has a function to transfer ERC20 token. There are many different scenarios where contracts, handling ERC20 tokens would run into this bug. One example is, that you would not be able to use decentralized exchanges that compiled its contract with solc ≥ 0.4.22 with your BadToken.

How could this happen?

This issue is clearly a bug in the token contract. The change in Solidity only brought this bug to daylight. One reason why there are so many BadTokens is that at one point OpenZeppelin implemented the wrong interface in their framework. Between 17 March 2017 and 13 July 2017 the interface was wrong:

https://github.com/OpenZeppelin/openzeppelin-solidity/blob/52120a8c428de5e34f157b7eaed16d38f3029e66/contracts/token/ERC20Basic.sol

So, most of the BadTokens used the faulty OpenZeppelin implementation of the ERC20 interface.

So now what?

There are two ways to fix this bug and I think these need to be approached in parallel.

On one hand, the teams of the affected token contracts need to fix their contracts. This could be done by redeploying the token contract or by updating the contract, if the token contract has some ability to update the contract programmed into it.

Another possibility is wrapping the token contract with a contract that calls the bad transfer function and returns a good transfer function. There are different proposals for such wrappers. e.g.

On the other hand, developers who are writing contracts that handle ERC20 tokens need to be aware of this bug, so they can anticipate the unexpected behavior of the BadTokens and handle them. This could be done by expecting the BadERC20 interface and checking the return data after the call to determine whether we called a GoodToken or a BadToken.

Conclusion

I think there is no reason to panic at the moment, because the updating cycle of decentralized exchanges and other big contracts is quite long. Nevertheless, this bug needs to be fixed as soon as possible.

There is also a very informative article about this issue from

Christian

. https://medium.com/@chris_77367/explaining-unexpected-reverts-starting-with-solidity-0-4-22-3ada6e82308c

You can read the issue on github here: https://github.com/ethereum/solidity/issues/4116

Thanks to

Christian

, Gérard and Eva for helping with this article

About lukas-berlin

I´m a solidity developer and auditor based in Berlin. I´m also the CTO of sicos.io and an organizer of the Solidity Berlin meetup. (www.solidity.berlin)

Join Coinmonks Telegram Channel and Youtube Channel get daily Crypto News

Missing return value bug — At least 130 tokens affected

Also, Read

Written by Lukas Cremer