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

Include list of non-standard ERC20 tokens #269

Merged
merged 6 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- [Code Maturity](./development-guidelines/code_maturity.md)
- [High-Level Best Practices](./development-guidelines/guidelines.md)
- [Token Integration Checklist](./development-guidelines/token_integration.md)
- [Known non-standard ERC20 tokens](./development-guidelines/non-standard-tokens.md)
- [Incident Response Recommendations](./development-guidelines/incident_response.md)
- [Secure Development Workflow](./development-guidelines/workflow.md)
- [Preparing for a Security Review](./development-guidelines/review_checklist.md)
Expand Down
56 changes: 56 additions & 0 deletions development-guidelines/non-standard-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Known non-standard ERC20 tokens

The following tokens are known to be non-standard ERC20 tokens. They may have additional risks that must be covered.

## Missing Revert

These tokens do not revert when a transfer fails, e.g. due to missing funds. Protocols that integrate these tokens must include a check for the transfer function's returned boolean success status and handle the failure case appropriately.

| Token | Notes |
| :--------------------------------------------------------------------------------------------------- | :---- |
| [Basic Attention Token (BAT)](https://etherscan.io/token/0x0d8775f648430679a709e98d2b0cb6250d2887ef) | |
| [Huobi Token (HT)](https://etherscan.io/token/0x6f259637dcd74c767781e37bc6133cd6a68aa161) | |
| [Compound USD Coin (cUSDC)](https://etherscan.io/token/0x39aa39c021dfbae8fac545936693ac917d5e7563) | |
| [0x Protocol Token (ZRX)](https://etherscan.io/token/0xe41d2489571d322189246dafa5ebde1f4699f498) | |

## Transfer Hooks

These tokens include [ERC777](https://eips.ethereum.org/EIPS/eip-777)-like transfer hooks. Protocols that interact with tokens that include transfer hooks must be extra careful to protect against reentrant calls when dealing with these tokens, because control is handed back to the caller upon transfer. This can also affect cross-protocol reentrant calls to `view` functions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also affect cross-protocol reentrant calls to view functions.

which can result in dirty reads?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "dirty reads"?


| Token | Notes |
| :----------------------------------------------------------------------------------------------------- | :---- |
| [Amp (AMP)](https://etherscan.io/token/0xff20817765cb7f73d4bde2e66e067e58d11095c2) | |
| [The Tokenized Bitcoin (imBTC)](https://etherscan.io/token/0x3212b29E33587A00FB1C83346f5dBFA69A458923) | |

## Missing Return Data / Transfer Success Status

These tokens do not return any data from the external call when transferring tokens. Protocols using an interface that specifies a return value when transferring tokens will revert. Solidity includes automatic checks on the return data size when decoding return values of an expected size.

| Token | Notes |
| :------------------------------------------------------------------------------------------ | :--------------------------------------------------------------------- |
| [Binance Coin (BNB)](https://etherscan.io/token/0xB8c77482e45F1F44dE1745F52C74426C631bDD52) | Only missing return data on `transfer`. `transferFrom` returns `true`. |
| [OMGToken (OMG)](https://etherscan.io/token/0xd26114cd6ee289accf82350c8d8487fedb8a0c07) | |
| [Tether USD (USDT)](https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7) | |

## Permit No-op

Does not revert when calling `permit`. Protocols that use [EIP-2612 permits](https://eips.ethereum.org/EIPS/eip-2612) should check that the token allowance has increased or is sufficient. See [Multichain's incident](https://media.dedaub.com/phantom-functions-and-the-billion-dollar-no-op-c56f062ae49f).

| Token | Notes |
| :-------------------------------------------------------------------------------------------- | :-------------------------------------------- |
| [Wrapped Ether (WETH)](https://etherscan.io/token/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2) | Includes a non-reverting `fallback` function. |

## Additional Non-standard Behavior

Additional non-standard token behavior that could be problematic includes:

- fee on transfers
- upgradeable contracts ([USDC](https://etherscan.io/token/0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48))
- tokens with multiple address entry-points to the same accounting state
- non-standard decimals ([USDC](https://etherscan.io/token/0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48): 6)
- non-standard permits ([DAI](https://etherscan.io/token/0x6b175474e89094c44da98b954eedeac495271d0f))
- do not reduce allowance when it is the maximum value
- do not require allowance for transfers from self
- revert for approval of large amounts `>= 2^96 < 2^256 - 1` ([UNI](https://etherscan.io/token/0x1f9840a85d5af5bf1d1762f925bdaddc4201f984), [COMP](https://etherscan.io/token/0xc00e94cb662c3520282e6f5717214004a7f26888))

Refer to [d-xco/weird-erc20](https://github.com/d-xo/weird-erc20) for additional non-standard ERC20 tokens.
4 changes: 4 additions & 0 deletions development-guidelines/token_integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ Token scarcity issues must be reviewed manually. Check for the following conditi
- [ ] **Users understand the risks associated with large funds or flash loans.** Contracts relying on the token balance must account for attackers with large funds or attacks executed through flash loans.
- [ ] **The token does not allow flash minting.** Flash minting can lead to drastic changes in balance and total supply, requiring strict and comprehensive overflow checks in the token operation.

### Known non-standard ERC20 tokens

Protocols that allow integration with arbitrary tokens must take care to properly handle certain well-known non-standard ERC20 tokens. Refer to the [non-standard-tokens list](./non-standard-tokens.md) for a list of well-known tokens that contain additional risks.

## ERC721 Tokens

### ERC721 Conformity Checks
Expand Down
Loading