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

EIPIP Meeting 106 #340

Closed
18 tasks
poojaranjan opened this issue Jun 6, 2024 · 6 comments
Closed
18 tasks

EIPIP Meeting 106 #340

poojaranjan opened this issue Jun 6, 2024 · 6 comments

Comments

@poojaranjan
Copy link
Member

poojaranjan commented Jun 6, 2024

Date and Time

July 03, 2024 at 17:30 UTC

Location

Zoom: TBA in the Discord #eip-editing channel

YouTube Recording: EIPIP Meetings

Agenda

1. Discuss Open Issues/PRs, and other topics

Edit to Final Proposals

Update to EIP-1

Misc Issue/ requires attention?

Call for Input

Call For Input Status Inclination/Result Deadline
343 Open July 28, 2024

2. Other discussions and updates from past meetings

3. EIPs Insight - Monthly EIPs status reporting.

Repo Issues PRs Total
EIP 11 76 87
ERC 4 71 75
RIP 1 11 12

(Dashboard)

4. EIP Editing Office Hour

  • Agenda 39 Video 38
    • 13/13 PRs Reviewed

5. Review action items from earlier meetings

Next Meeting date & time

July 17, 2024 at 17:30 UTC

@poojaranjan poojaranjan mentioned this issue Jun 6, 2024
6 tasks
@Dexaran
Copy link

Dexaran commented Jun 28, 2024

I would like to attend the EIP editors meeting to discuss security issues with ERC-20 and ERCs in general. I was raising this concern some time ago, there was some brainstorming done (https://ethereum-magicians.org/t/modification-of-eip-process-to-account-for-security-treatments/16265/26 and this https://hackmd.io/@SamWilsn/HJL1ydMVp) but there was no outcome and it seems this topic is not moving.

So I would like to:

  • Highlight the existing challenges of the EIP process from a security perspective.
  • Hear EIP editors feedbacks
  • Discuss future steps that could be taken in order to solve any of the mentioned problems
  • Discuss the state of ERC-20 "Known issues". Over a year ago we discussed the possibility of adding a warning and my proposal was voted out by @gcolvin if I'm correct. A year passed, the issue is not solved, the amount of lost funds increased. May be it requires different approach.

@frangio
Copy link

frangio commented Jul 1, 2024

Consider discussing how to resolve the EIP-7212 migration as it is still up and appears valid even though the current spec is RIP-7212. Among other things the precompile address is different from the one in RIP-7212 and i'm concerned someone might mistake it for the true spec.

@Dexaran
Copy link

Dexaran commented Jul 3, 2024

There is a known problem of ERC-20 standard which resulted in a loss of $83,000,000 worth of tokens in smart-contracts with verified source codes on Ethereum and another $150,000,000 are most likely lost in other contracts.

I discovered this problem in 2017 and I reported it multiple times.

So, I think that considering the results of the implementation of @gcolvin 's proposal regarding handling problem disclosures in EIP process - the strategy needs to be changed.

"Final = no change" rule resulted in a situation where an issue which is known for 7 years caused $80M to $230M damage to the ecosystem and that issue is still not fixed even though a number of solutions was proposed years ago.

The following points are required to actually solve the problem (by "solve" I mean (1) reduce the number of pure ERC-20 contracts that will be deployed with this issue in the next years and (2) reduce the amount of funds that will be lost because of this issue in Ethereum ecosystem. It is possible to mitigate a significant part of damage without breaking compatibility with ERC-20 standard, but implementors need to know about it):

  • Exposure. The problem must be clearly communicated to the implementors. The severity of the problem must be clearly communicated as well. If something is labelled as a "known issue" devs tend to treat it as something they don't need to worry about.

  • Alternatives. Possible solutions must be communicated to the implementors as well. It is possible to implement additional restrictions for transfer function that will mitigate a significant portion of damage that this issue could otherwise cause with a pure ERC-20 implementation.

  • Upgrading processes. If there are alternatives then they need to be communicated to the implementors as well and the migration process must be coordinated. I've created a ERC-7417 that would define an upgrading process between ERC-20 and ERC-223 standards. The same can be created for any of the existing standards. I think that including the "upgrading process" ERC in the text of each existing token standard is quite important if one exists. EIP process does not allow for it currently.

Those requirements are not my personal invention, but based on the documents describing C++ standards evolution and security issue reporting practices.

https://www.stroustrup.com/hopl-almost-final.pdf

https://docs.github.com/en/code-security/security-advisories/guidance-on-reporting-and-writing-information-about-vulnerabilities/about-coordinated-disclosure-of-security-vulnerabilities

A good example of how implementors may treat a "known issue" is OpenZeppelin:

The current pure ERC-20 implementation in their repo is directly affected by the described issue and if you would copy & paste contracts from their repo then it would inevitably cause the holders of these tokens to lose funds as it happened with other tokens of this standard already https://github.com/OpenZeppelin/openzeppelin-contracts/tree/5480641e5c572fc7f9d68d59003f4b6417168cdd/contracts/token/ERC20

Providing a description of issues as well as the recommendations on how this issues can be mitigated, solved or avoided is important. Providing the solution and communicating it to the implementors may be even more important than disclosing the issues on it's own.

My proposals

  1. I insist that the proposal to add ERC-20 problem description to the text of ERC-20 and include the possible ways of solving that problem are a better option than what @gcolvin proposed. Therefore I would recommend to change the "final = no change" rule to "final applies to the specification of the EIP text, security considerations can be appended". It is absolutely not possible to identify and describe all the software issues once and for all.
  2. I would like to revive Add EIP: Security Flaws in ERC-20 Transfer Patterns ethereum/EIPs#7915 and push it to the "final status" as an informational EIP. It can be linked from the "Security Considerations" section of ERC-20.
  3. The development must be driven by real problems. Add a rule "if some problem caused a financial loss and it can be verified through the transaction history in the blockchain then the description of the problem can be added to the corresponding EIP security considerations". This would remove the burden of judging whether something is a security issue or not from EIP editors and allow for communicating the most important information to the implementors.

@SamWilsn raised a question whether it is necessary to add issues like "you can send a ERC-XXX token to a wrong address and therefore lose your money" to the security considerations sections of EIPs. I would like to address this claim in a separate comment below.

@poojaranjan
Copy link
Member Author

Summary

1. Discuss Open Issues/PRs, and other topics

ethereum/EIPs#8590

ethereum/EIPs#8634

  • Was waiting on 3rd editor's approval. @xinbenlv approved it and is merged.

ethereum/ERCs#473

  • Keep the issue open. It could be a good Issue to be worked on by someone willing to make open-source contribution.

ethereum/ERCs#243

  • Keep the issue open. Need to be worked on.

Spam Issues - Closed

2. Other discussions and updates from past meetings

Security issues with ERC-20 and ERCs in general. Ref:

#340 (comment)

  • @Dexaran provided an overview of the issue and proposed solutions. Ref:Comment
  • To follow the conversation, follow the recording.

Discuss blockers to merge a Pull Request

@poojaranjan shared main reasons identified for open PRs

  • Waiting for Editor Consensus: Many pull requests are pending agreement among the editors.
  • Waiting for Author Review: Some proposals require the original authors to review and approve changes.
  • Draft Status: Some EIPs are still in draft form and need further development before they can be merged.
  • CI Tests Pending: Several proposals are waiting for Continuous Integration (CI) tests to pass.
  • Client devs are looking for 'Draft` PRs to be merged sooner to move the discussion to the FEM page rather than on the PR itself.
  • @SamWilsn agreed to the idea of merging Draft PRs
  • @xinbenlv also recommended the same for ERC proposals
Estd. timeline for merging Draft proposals
  • @SamWilsn demoed a tool that can help identify the open PRs to the editor in order of last engaged with the author.
  • This will help get an estimated timeline by which the PR will be reviewed by an editor. The idea is to add this tool to the eips.ethereum.org.
  • Editors will look into open PRs added to the agenda and merge them.

Also, the group discussed merging the Draft PR with a minimum requirement. Most of the editing reviews will be done when the EIP is moved to Review status.

@poojaranjan poojaranjan mentioned this issue Jul 4, 2024
10 tasks
@poojaranjan
Copy link
Member Author

Closing in favor of #346

@Dexaran
Copy link

Dexaran commented Jul 5, 2024

@poojaranjan I've updated the comment #340 (comment)

Links were provided, my proposals are stated.

@SamWilsn

"Do we need to add issues like sending to wrong address to the text of each ERC describing a token that can be sent to wrong address?"

Sending to wrong address is a protocol-level issue. It is related to what is considered a valid "account" in Ethereum. Considering any pattern of 42 hexadecimal symbols as a valid transaction recipient is not the best idea. In theory the ENS was designed to solve it but it failed to gain sufficient adoption.

There are alternative implementations. For example in EOS there is a built-in naming service and this is how a EOS account looks like https://www.bloks.io/account/dexaran.x https://www.bloks.io/account/dexaraniiznx

This is how a contract looks like in EOS https://www.bloks.io/account/tethertether?loadContract=true&tab=Tables&account=tethertether&scope=tethertether&limit=100

And if you will try to access an EOS account that doesn't exist (i.e. it is not assigned to any public key) then you will not be able to do this https://www.bloks.io/account/dexarannn404

Similarly, if you would try to send funds to a "non-existing account" in EOS - the transaction will fail.

Is sending to wrong address a problem? - Yes, it is. In my opinion it can be classified as a security problem because it threatens the safety of users funds. But this is a different problem than the one I was reporting earlier. And it is a protocol-level problem.

Is it worth adding a protocol-level problem to every text of every EIP which is affected? - In my opinion no. If there is a EIP that describes how addresses are derived from a private key in Ethereum then it would be the right place to mention this problem.

If you would like to completely remove the burden of making this types of decisions from EIP editors then I would support it either. Anyways it's better to have a protocol-level issue described in every token ERC and ERC-20 issue described in ERC-20 text than not to have any issues described at all.

I would propose to implement an anti-spam threshold i.e. "If there are more than X amount of $ lost because of a certain problem then it must be added to the security considerations section of affected EIPs/ERCs" so that to avoid describing spam issues that pose no real threat. For example you can deliver Ether to a smart-contract via SELFDESTRUCT bypassing the fallback function invocation but this issue caused about $0 losses to the users of the ecosystem during the past 7 years unlike the ERC-20 issue which caused about $80M-$230M.

P.S. The same applies to counterfactual contracts. Before the contract is created it mustn't be considered an entity capable of receiving a financial transaction. This bypasses fallback function invocation and must be considered an anti-pattern in general. Ether can also be lost this way and it's a similar issue of "account generation".

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

4 participants
@frangio @Dexaran @poojaranjan and others