Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EIP: Security Flaws in ERC-20 Transfer Patterns #7915

Closed
wants to merge 16 commits into from
Closed
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions EIPS/eip-7915.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
eip: 7915
Dexaran marked this conversation as resolved.
Show resolved Hide resolved
title: Security flaw disclosure in ERC-20 workflow
description: Disclosure of a security flaw in ERC-20 transferring method that caused a loss of $201,690,741 as of 17.08.2023.
author: Dexaran (@Dexaran) <dexaran@ethereumclassic.org>, Vladimir Vencálek <vladimir@callisto.network>, Yuriy Kharytoshin (@yuriy77k) <yuriy@callisto.network>, Laurent Riche (@spatialiste) <tonton@callisto.network>
discussions-to: https://ethereum-magicians.org/t/disclosure-of-a-security-flaw-in-erc-20-transferring-workflow/16249
status: Draft
type: Informational
created: 2023-10-24
requires: 20, 1363
---

## Abstract

The following describes a security flaw in the transferring workflow of [ERC-20](./eip-20.md) token standard. It must be taken into account that all token standards that declare full backwards compatibility with [ERC-20](./eip-20.md) also inherit this security flaw, for example [ERC-1363](./eip-1363.md).

## Specification

### [ERC-20](./eip-20.md) design overview

[ERC-20](./eip-20.md) standard declares two methods of transferring tokens: (1) `transfer` function and (2) `approve` & `transferFrom` pattern. `approve` & `transferFrom` is supposed to be used to deposit tokens to contracts. The `transfer` function is supposed to be used for transfers between externally owned addresses however this is not directly written in the specification. If the tokens are sent to a contract address via the `transfer` function then the recipient contract will not recognize the depoist.
Dexaran marked this conversation as resolved.
Show resolved Hide resolved

### Security flaw

**The `transfer` function does not notify the recipient of an incoming transaction which makes error handling impossible.** Error handling is an essential part of secure software development. If tokens are sent to any contract via the `transfer` function and the recipient contract does not support extraction of tokens (i.e. it doesn't implement any functions which would allow to send tokens out) then it is a clear case of user error that must be reverted. For example if a user would send plain ether to a contract that does not explicitly declare that it is intended to accept ether deposits then such transaction would be reverted automatically. In case of [ERC-20](./eip-20.md) tokens a user can push the token contract into incorrect state where a user no longer controls the tokens by picking a wrong function when performing a transaction.

**A burden of determining the method of transferring tokens is placed on the user in a situation where one option is obviously wrong and will result in a loss of funds.** Prompting a user to make a decision on the internal logic of the contract combined with the lack of an implementation of error handling for users actions is another security failure that can result in incorrect token contract behavior and a loss of funds for the end user.

### Token losses

As of 17.08.2023 $201,690,741 worth of [ERC-20](./eip-20.md) tokens were lost in top 50 (measured by transactions count) token contracts due to the described problem of the standard.

## Rationale

Security flaw disclosures are an important part of software development. Increasing awareness of the problem helps the development community to implement solutions and minimize the damage that a particular flaw can deal to the users.

## Security Considerations

- The best option is to avoid using [ERC-20](./eip-20.md) tokens and to upgrade the ecosystem to a more secure token standard that (1) **properly supports error handling and notifies recipients** of incoming transfers, (2) **automatically determines the transferring method** if token transfers to externally owned accounts and to contracts are performed differently without prompring a user to make any decisions that can potentially push the contract in incorrect state or result in any errors, (3) **abstracts the user from the internal logic of the contract**.
- It could be possible to apply a patch on [ERC-20](./eip-20.md) standard by disallowing the use of `transfer` function to deposit tokens to contracts. This would introduce a breaking change to the "final" standard and therefore is not possible according to EIP process.
- Any implementor of a [ERC-20](./eip-20.md) token must be aware of the issue and describe it in the most visible form. Developers and users MUST be notified of an issue.
- Any developer of a UI that operates with tokens MUST perform additional checks when a user is transferring tokens via `transfer` function. If the recipient is a contract then the user must receive a wraning message stating that he/she is transferring tokens to a contract directly and it can result in tokens being stuck there without an option to recover them. It is a common user mistake that (for some reason) can't be prevented on the contract level. Ideally the UI must also check if the recipient is a contract that is known to not be intended to receive tokens such as a contract of another token, ENS contract or most other contracts except multisigs.
- Any contract deployed on Ethereum mainnet MUST expect that there would be "stuck" ERC-20 tokens accidentally deposited to the contract address. A special function to withdraw tokens MUST be implemented.

```
function rescueERC20(address _token) onlyOwner external
{
amount = IERC20(_token).balanceOf(address(this));
IERC20(_token).transfer(msg.sender, amount);
}
```

## Copyright

Copyright and related rights waived via [CC0](../LICENSE.md).