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

Integrate ZRC-223 standard for ZRC20 #107

Open
Dexaran opened this issue Dec 8, 2023 · 4 comments
Open

Integrate ZRC-223 standard for ZRC20 #107

Dexaran opened this issue Dec 8, 2023 · 4 comments

Comments

@Dexaran
Copy link

Dexaran commented Dec 8, 2023

ERC-20 standard security flaw caused millions of dollars losses

ERC-20 token standard has a major security flaw that I disclosed to Ethereum Foundation in 2017 but they ignored it.

ERC-20 does not implement "transaction handling" for transfer function and it makes it impossible to handle erroneous transfers OR deposit tokens to contracts using the transfer function i.e. if a user sends Ether, an ERC-721 NFT token, or an ERC-223 token to a contract that does not explicitly declare that it is supposed to receive them, then the tokens or Ether are rejected and the transaction automatically fails. This doesn't work with ERC-20 - if a user sends ERC-20 to some random contract then the transaction will succeed and the tokens become "frozen" at the contract's address with no way to recover them. As of today, the described problem caused $90,000,000 to $200,000,000 financial damage to users of Ethereum tokens.

Why ERC-20 is designed the way it is

There was a bug in EthereumVM at the time when ERC-20 was developed. Pull transacting method was introduced to make tokens not affected by this bug. It was a quirk that addressed the earlydays bug in the VM, not a smart design.

  • ERC-20 standard was proposed in 2015. At this time there was a bug in Ethereum Virtual Machine 1024 call stack depth. The approve & transferFrom method of the ERC-20 standard was introduced so that this bug does not affect tokens.

  • In Tangerine Whistle hardfork the call stack depth problem was solved. This happened on block 2463000 in 2016. At this time approve+transferFrom method became irrelevant and the ERC-20 standard should have been considered deprecated.

Here is a @vbuterin 's comment regarding the situation with token standards and call stack depth problem:

67496b44-afc0-4acc-bc1b-bafb9be43e5c

approve & transferFrom are unnecessary now and they introduce security risks as well as they authorize third parties to operate with users funds. This is not as critical as the lack of error handling with transfer func however.

Source: ethereum/ethereum-org-website#10854 (comment)

ZRC20 is affected

ZRC20 implementation is prone to the same exact issue and will inevitably cause financial damage to your customers if it is not fixed:

https://github.com/zeta-chain/protocol-contracts/blob/main/contracts/zevm/ZRC20.sol#L178-L189

Alternative token standard support proposal

I've designed an alternative ERC-223 token standard to solve this exact problem of ERC-20 security bug and it was finalized in 2023.

Obviously ERC-20 will not go away overnight, but I'm building the ecosystem for ERC-223 tokens currently. I believe that in the long run the standard that does not burn users money will thrive. The main idea is to make ERC-20 and ERC-223 interoperable for now and let a dual standard ecosystem settle.

I'm ready to contribute to the development of the safer token standards. We can support Zeta testnet with our token converter. I can write example ERC-223 (or ZRC-223) contracts to demonstrate an alternative way of implementing tokens on Zeta Chain.

We can deploy Dex223 on Zeta chain.

@lumtis
Copy link
Member

lumtis commented Dec 9, 2023

Hi @Dexaran thank you for the report.
However, the notion of "critical security flaw" seems over-exaggerated.
The link you posted mention about logical flaw.
Can you explain how you would consider ERC20 implementation as a security flaw?

@Dexaran
Copy link
Author

Dexaran commented Dec 9, 2023

Can you explain how you would consider ERC20 implementation as a security flaw?

It falls in "critical severity security vulnerability" criteria in OpenZeppelin bug bounty.

255351496-0246fb67-bf3d-494e-97f4-1efcff64a06e

OpenZeppelin/openzeppelin-contracts#4451

@Dexaran
Copy link
Author

Dexaran commented Dec 9, 2023

Again, ERC-20 implementation violates some well-known principles of secure software development.

  • Failsafe defaults. With ERC-20 defaults are not failsafe and the default behavior of the transfer function is to perform a transfer of value without checking for its correctness.
  • Error handling is a must. The ERC-20 is designed in a way that does not allow for error handling. As a result, tokens are “lost” due to obviously erroneous transfers, which could be easily prevented if a proper communication model would be implemented. When you send Ether to contract the recipient is notified automatically which allows to handle errors.

https://owasp.org/www-project-developer-guide/draft/04-foundations/03-security-principles

Violating some widely adopted and well-documented principles of secure software development will result in your software not being secure (quite logical). Hence millions of dollars are lost due to the ERC-20 flaws that I described.

I'm not saying that your standard has a security vulnerability and someone will hack it. I'm saying that your token standard (as well as the base ERC-20 standard) is not secure by default and it will result in your users losing money without a hack.

@Dexaran
Copy link
Author

Dexaran commented Dec 9, 2023

You can probably apply a "bandage" to ZRC-20 by prohibiting direct transfers to smart-contract addresses via transfer function. This will require you to redefine some part of the Uniswap logic however as it does unchecked transfers via transfer func.

See contracts below

(You can optimize the gas usage by issuing an unlimited approval at the moment of the pair contract creation to avoid multiple approve calls)

@lumtis lumtis changed the title Critical security flaw in ZRC20 token Integrate ZRC-223 standard for ZRC20 Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants