Forbid user-defined reentrancy as a condition of ERC20 badge

Proposal

Rename the ERC20 badge to ‘classic ERC20’.

Add the following requirement to the ERC20 badge:

  • Token contracts whose transfer or transferFrom function allows the caller, sender or receiver to execute smart contract calls are not acceptable even if they are ERC20 by the standard definition. In particular ERC777 tokens are not acceptable.
  • Token contracts charging a transfer fee to the sender or the recipient are not acceptable. Token contracts which could charge a fee, but do not due to current parameters, are acceptable.

Motivation

Name change

The name change is required in order to avoid implying that some contracts which do not fulfill all badge requirements are not ERC20.

Reentrancy

Some token contracts like imBTC allows the caller, sender or receiver to execute calls during a transfer. Even if this behaviour may be wanted by the token contract developer, it is often unexpected by creator of contracts interacting with tokens and can lead to loss of funds.

Recently the imBTC pool of Uniswap was hacked by an attacker exploiting the Uniswap contract. Fortunately the uniswap.ninja frontend using the list of tokens with ERC20 badges in the T2CR did not list imBTC as it did not request a ERC20 badge so no user were affected. However someone could request it a badge which could lead to loss of user funds.
A similar exploit was used to hack dForce with the hacker stealing for 25M$ of cryptoassets (which he has then given back as the investigation was getting closer to him).

I do understand that it may not seem “fair” to prevent ERC777 from getting a badge due to errors of other smart contract developers. However this is currently the most practical way to let applications continue to use the ERC20 badge. In case there is demand a ERC777 badge could be created. Also note that at the time of writting adding this rule would not result to any token losing its ERC20 badge.
Also note that contracts created by the Kleros do not have any problem interacting with ERC777.

Transfer fees

Token contract charging transfer fees cause problems in multiple smart contracts. The first users to withdraw tokens from them will get their full amounts, but the contract will become insolvent for the last users due to the fees which may see a part or all of their tokens lost. Whether all those are ERC20 is subject to debate.
From the standard 'Transfers _value amount of tokens to address _to ’ tokens charging the fee to the recipient are not ERC20 so already covered by the current criteria. However for those charging a fee to the sender, I see arguments for both sides and we may want to also specify that charging fees for transfer excludes the token from the ERC20 badge.
Specifying it in the badge requirement solves this debate and protects users of dapps pulling their list of tokens from the T2CR from losing their money.

1 Like

Seems like a good idea to me, however the name ERC20 badge might become a bit confusing. Maybe rename to a more universal Exchange badge or Uniswap.ninja badge.
Another option is to create a new badge for Buggy or Exploitable tokens.
I guess the question becomes whether we want the name ERC20 badge to refer to compliance with the ERC20 standard or just as an cosmetic name that is somewhat compliant and randomly used.

2 Likes

In the case of imBTC the token is not buggy in itself and is ERC20 per its definition. But it seems that developers don’t often assume that ERC20 tokens can reenter.
Another potential exclusion of ERC20 badges would be tokens which charges the transfer fee. From the standard 'Transfers _value amount of tokens to address _to’ tokens charging the fee to the recipient are not ERC20 so already covered by the current criteria. However for those charging a fee to the sender, I see arguments for both sides and we may want to also specify that charging fees for transfer excludes the token from the ERC20 badge.

Maybe renaming it to ‘Classic ERC20’?

There is also the question of tokens providing dividends and interests. In those, interests and dividends, but not the principal, could be stuck in smart contracts holding those. For those the risk is lower as users would effectively get the right amount of tokens so there seems to be very little risk in accepting those.

I amended the proposal to make clear that contracts charging transfer fees are not ERC20.