Skip to content

Instantly share code, notes, and snippets.

  • Star 20 You must be signed in to star a gist
  • Fork 6 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save Dexaran/ddb3e89fe64bf2e06ed15fbd5679bd20 to your computer and use it in GitHub Desktop.
ERC20 token standard vulnerability classification.

Previously described at: ERC20 critical problems medium article.

Description.

ERC20 is the most common Ethereum token standard. It should be noted that it is also the first Ethereum's token standard as well.

It is also important that the original ERC20 proposal is a definition of token interface. EIP20 does not define a reference implementation for this token standard. Here is OpenZeppelin implementation of ERC20 token: https://github.com/OpenZeppelin/zeppelin-solidity/tree/master/contracts/token/ERC20

ERC20 token standard implementation assumes two ways of token transferring: (1) transfer function and (2) approve + transferFrom pattern.

The transfer function does not notify received that the token transfer happened. ERC20 token standard lacks even handling/communication model. This is a critical flaw of the standard that led to impossibility of error handling.

Also depositing tokens to a contract with transfer function is an intuitive way of depositing crypto assets for users, however this will result in an unhandled error in case of ERC20 tokens which will cause a permanent loss of tokens if there is no specific function for extraction of the stuck ERC20 tokens in the recipient contract.

It should be noted that event handling is a well-known and standard practice in programming. In case of token standard, a token transfer should be considered an event. transfer function of ERC20 does not provide any possibility to handle the transfer, i.e. it is silently increasing a balance variable of the receiver. It is impossible for the receiver to recognize that transfer occurs if the receiver is a contract. As the result, the only correct way to make a token deposit to a contract is an approve + transferFrom pattern.

In case of a mistake, the transfer function MUST throw an error and revert a wrong transaction. Otherwise it will cause negative consequences (lost tokens) for end user. This is a very common and default practice in programming, called Exception Handling: https://en.wikipedia.org/wiki/Exception_handling

This has already led to the loss of millions of dollars for the whole Ethereum ecosystem at the moment. Every contract on Ethereum is a potential trap for ERC20 tokens (read ENS contract became token holder).

How it is causing money losses.

Expected transaction behavior: a transaction is executed and it is resulting in value transfer. In case of error, transfer of value will not occur.

This works exactly as expected in case of Ether transfers. If you send Ether to a contract that is not intended to work with Ether then the transaction will be rejected by the recipient smart-contract and the transfer of value will not occur.

This does not work as expected in case of ERC20 token. The transfer of tokens (i.e. transfer of value) will not be reverted or rejected by the recipient smart-contract due to lack of possibility to handle the transfer function invocations. This is causing unexpected transfer function behavior and produce unexpected result i.e. loss of funds for end users.

I often heard how developers declare (1) "Not a bug, user mistake" or (2) "This is not a bug or a security vulnerability, this is a specific feature of ERC20 standard design".

I'll explain why the described property of ERC20 standard is exactly a bug below.

REMINDER: Also, if a "specific feature of software design" has caused losses of millions of dollars for software users, then it is an awful design at least.

ERC20 token standard bug explanation.

Software bug definition: https://en.wikipedia.org/wiki/Software_bug

I will quote this:

A software bug is an error, flaw, failure or fault in a computer program or system that causes it to produce an incorrect or unexpected result, or to behave in unintended ways.

The intention of transfer function is to transfer tokens.

This is how the transfer function is specified at the ERC20 token standard:

transfer
function transfer(address _to, uint256 _value) returns (bool success)

Send _value amount of tokens to address _to

There is no phrase like "If you send your tokens to the address or a contract that is not intended to work with tokens then your tokens are permanently lost." We should keep in mind that we are talking about the Ethereum smart-contracts. In case of Ether transaction, every time a transaction is invoked incorrectly (a receiver is unable to handle transaction or a receiver is a contract that is not intended to work with Ether), an error is thrown and the transaction is reverted.

Thus, the expected behavior is that in the event of an error caused by the recipient's inability to handle the incoming transaction, such a transaction should fail.

Ethereum users are often familiar with Ether transactions. As the result, the expected behavior is that if a transfer of tokens is invoked incorrectly (a receiver is unable to handle transaction or a receiver is a contract that is not intended to work with Ether tokens), an error must be thrown.

The statement "... produce an incorrect or unexpected result, or to behave in unintended ways", which is a property of a program error, is of decisive importance here. The "bug of ERC20 transfers" is the unexpected behavior of transfer function in this case.

Some developers could argue that these are my personal assumptions but let's dive deeper into Software Vulnerabilities definition first: https://en.wikipedia.org/wiki/Vulnerability_(computing)#Software_vulnerabilities

I will quote this:

Common types of software flaws that lead to vulnerabilities include: ... Blaming the Victim prompting a user to make a security decision without giving the user enough information to answer it.

The statement "... prompting a user to make a security decision without giving the user enough information to answer it. " which is a property of Software Vulnerability, is of decisive importance here.

There is no sufficient information at the ERC20 token standard definition about the behavior of transfer function in case of the error-handling. A phrase "WARNING: If you will call this function to transfer your tokens to any contract then your tokens are permanently lost." MUST be added to the definition of transfer function of the ERC20 token standard.

A phrase "Calling the transfer function to transfer your tokens to contracts is prohibited in this token standard." MUST be added to the Abstract section of the ERC #20 token standard.

This is required to make each token and UI developer aware of what they are working with. This is what is missing at ERC20 standard.

As soon as this will be implemented, each token developer and UI developer MUST place a big red banner "WARNIGN: You are attempting to use ERC20 token, you should know that if you call a transfer function to send tokens to a contract then your tokens are permanently lost." Otherwise it will be a Software Vulnerability: User Interface fault (https://en.wikipedia.org/wiki/User_interface).

Lost tokens computed at 27 Dec, 2017.

  1. QTUM, $1,204,273 lost. watch on Etherscan

  2. EOS, $1,015,131 lost. watch on Etherscan

  3. GNT, $249,627 lost. watch on Etherscan

  4. STORJ, $217,477 lost. watch on Etherscan

  5. Tronix , $201,232 lost. watch on Etherscan

  6. DGD, $151,826 lost. watch on Etherscan

  7. OMG, $149,941 lost. watch on Etherscan

  8. STORJ, $102,560 lost. watch on Etherscan

These are only 8 token contracts that are shown as an example. Each Ethereum contract is a potential token trap for all the ERC20 tokens, thus, there are much more losses than I showed here.

Such contracts that are designed to work with tokens are at very danger zone: state channels, decentralized exchanges, payment services that accept tokens.

Conclusion.

Let me summarize what is stated above. The specific feature of ERC20 token standard design that caused millions of dollars loss for token users IS a Software Bug and it can be classified as Software Vulnerability.

The specific design of ERC20 token standard functionality is producing unexpected result and causing unexpected behavior which is a property of software bug.

As ERC20 token standard is a smart-contract interface specification, then I can say that Interface fault, such as Blaming The Victim is taking place here. This is also a property of software vulnerability.

Saying "This is not a bug" is absolutely wrong. Definitely, this is a bug and a vulnerability. Saying "This is a user mistake" is a kind of Blaming the victim.

@gnostication
Copy link

Thank you, once again, for at least trying to get the Ethereum community to wake up.

ETC is the hope, once the hype of ETH dies.

@SL50
Copy link

SL50 commented Feb 12, 2018

It is helpful to name token standards that inherit this vulnerability of ERC20. ERC721 and ERC644 are affected AFAIK.

@Dexaran
Copy link
Author

Dexaran commented Feb 13, 2018

This is a good point.
Many of token standards are just the extensions of ERC20, thus they inherit this bug.

This token standards are affected by ERC20 bug

ERC-644: ethereum/EIPs#644

ERC-677: ethereum/EIPs#677

ERC-721: ethereum/EIPs#721

ERC-827: ethereum/EIPs#827

ERC-875: ethereum/EIPs#875

ERC-1363: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1363.md

This token standards are NOT affected by this bug

ERC223: ethereum/EIPs#223

ERC223 token standard is specially designed to resolve this problem. As the result, it overrides the transfer function and explicitly defines its behavior. ERC223 token can not be affected by this bug.

ERC777: ethereum/EIPs#777

ERC777 token standard uses completely different function signatures. It depends on implementation whether a token of ERC777 standard will implement this bug or not. ERC777 standard does not inherit it by default but it does not implement any mechanism to prevent it, thus it is not guaranteed that ERC777 tokens are secure.

@gtabmx
Copy link

gtabmx commented Oct 25, 2018

This isn't a bug insofar as sending an email to an unintended or wrong recipient is, or sending the only copy of your English will to a person in Japan who doesn't speak English. Poor user experience: yes. Bug: no.

Consider this user flow:

  1. User chooses username for a new social media site.
  2. Last registration step is to provide an email where your welcome letter will be sent, as well as your temporary password.
  3. After typing in the email address and clicking submit, the email is fired off, but does not give you the ability to re-send to another address.

If you typed the wrong email or an email you don't have access to, its over. This is not a bug. This is a poor and not properly thought out user experience. The architect and UX designer failed, but the software developer did not.

Calling this "ERC20 transfer to a recipient that cannot speak ERC20" issue a bug is erroneous.

@Dexaran
Copy link
Author

Dexaran commented Mar 9, 2019

@gtabmx as I wrote in the article, this is definitely a bug. User does not have enough information to decide whether the contract is sending to a wrong address or not. And the contract does not throw an error when processing the "known-to-be-incorrect" transaction.

Error handling is a very basic thing. Failure to implement the very basic musthave feature is a security issue. If you will not place "onlyOwner" restriction on a governance function for example it would be a security issue.

As the result, millions of dollars are lost.

@vineetchawla
Copy link

I believe completely that this is a bug. Leaving out error handling for an incomplete transaction is a software developer issue. I just came across this while creating an ERC20 and ERC721 marketplace. Will definitely keep this in mind now.

@nicszerman
Copy link

The gas saved by skipping "event handling" is greater than the funds lost by accident. Moreover, AFAIK this is intended behavior and therefore not a bug.

@Dexaran
Copy link
Author

Dexaran commented Sep 14, 2023

The gas saved by skipping "event handling" is greater than the funds lost by accident.

The amount of lost funds was at least $201,690,000 (in 50 checked contracts out of 1300 existing).

Imagine a DAPP gets hacked and $200M are stolen but the dev team replies "We decided $200M is not that much so we will not fix the security issue that was exploited to save gas".

Moreover, AFAIK this is intended behavior and therefore not a bug.

If it is intended that your customers lose $200M due to software failures then the developer is a criminal.

@nicszerman
Copy link

nicszerman commented Sep 14, 2023 via email

@Dexaran
Copy link
Author

Dexaran commented Sep 21, 2023

I also maintain that since current behavior is intended
(AFAIK), then it’s not a bug.

If it was intended that customers will lose millions of dollars then the developer is a criminal.

About $8B are spent per year in gas fees in Ethereum Mainnet. By far the
most common operation are ERC20/ERC721 transfers. If this event increases
gas cost by 20% and 15% of operations are ERC20/ERC721 transfers, that’s
$240M per year.

ERC-223 transfers can be a bit more expensive than ERC-20 but it's all about scalability issues that are solvable. Security issues however will not automatically diminish with time if someone will not solve them. The $200M lost today can become $200B lost tomorrow, nothing prevents it.

Follow the trend: in 2017 there were $16K lost, in 2018 $1M lost, in 2023 $200M lost.

At the other hand ERC-223 swaps are 15% cheaper than ERC-20 swaps (due to the elimination of approvals) and are much more secure than approve&pull pattern.

@nicszerman
Copy link

nicszerman commented Sep 21, 2023

Your first point about criminal behavior is very weak. The second point about funds lost increasing over time misses that the gas savings will increase equally. Your third point about gas being cheaper than calculated due to avoiding approvals is good.

My standing now is that your proposal and ERC20 should coexist and each app will choose according to use-case. All tokens could be wrapped in ERC223 so defi users could mostly hold ERC223 while currency-payments users could use ERC20.

@Dexaran
Copy link
Author

Dexaran commented Sep 22, 2023

My standing now is that your proposal and ERC20 should coexist

Personally I would just consider ERC-20 deprecated and insecure and get rid of it. But I understand that the ecosystem built around it is huge and it will not be gone in a blink of an eye.

Your scenario of two token standards co-existing is the most realistic.

I have developed EIP 7417: Token Converter - a service that will convert ERC-20 tokens to ERC-223 or ERC-223 back to ERC-20.

ethereum/EIPs#7418

The UI is still in development https://gorbatiukcom.github.io/token-converter/

Any feedbacks are always welcome anyways.

@nicszerman
Copy link

@Dexaran
Copy link
Author

Dexaran commented Oct 2, 2023

ERC-777 is prone to exactly the same issues as ERC-20. It's "backwards compatibility" in fact means "it inherits security flaws of ERC-20 and will result in the same financial losses for the end user".

As long as you place a burden of determining which transacting logic to execute on a user instead of doing it automatically (at the contract level) - the problem will persist.

Transaction type ERC-223 Ether ERC-20 ERC-721 (NFT) ERC-777 ERC-1155 ERC-1363 EOS C++ token
Push tx + + - + + + + +
Pull tx (risky) - - + + + + + -
Unhandled (insecure) - - + - + - + -

@nicszerman
Copy link

nicszerman commented Oct 3, 2023 via email

@Dexaran
Copy link
Author

Dexaran commented Oct 4, 2023

“Furthermore, since contracts are required to implement these
hooks in order to receive tokens, no tokens can get stuck in a contract
that is unaware of the ERC777 protocol
, as has happened countless times
when using ERC20s.”

This is simply not true.

@nicszerman
Copy link

nicszerman commented Oct 4, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment