SamWilsn
commented
11 months ago Can we add "Placing a warning on ERC-20" (from here) on this agenda?
Closed poojaranjan closed 11 months ago
SamWilsn
commented
11 months ago Can we add "Placing a warning on ERC-20" (from here) on this agenda?
poojaranjan
commented
11 months ago @SamWilsn added !
SamWilsn
commented
11 months ago @Pandapip1 would like to remove the requires: header in favour of an automatically generated bibliography:
SamWilsn
commented
11 months ago @Therecanbeonlyone1969 would like to add NIST as a permitted origin for external links.
poojaranjan
commented
11 months ago Final or an existing EIP.https://github.com/ethereum/EIPs/pull/7468 Ref Discord
Informational and add a warning/ announcement to move it to Meta EIP. Informational and merge the PR. In the next call, there will be another PR to change the type to Meta to enforce the changes as mentioned in the EIP.No objection
No objection
Last Call.
Dexaran
commented
11 months ago @poojaranjan @SamWilsn @gcolvin
https://github.com/ethereum-cat-herders/EIPIP/issues/255#issuecomment-1671818828
Most of the editors on call weren't in favor of adding a warning to a Final or an existing EIP.
After some discussion, the general agreement was based on @gcolvin's suggestion that the user can write an informative EIP to share information about the usage of EIP-20 or use other media for information sharing at his/her disposal.
No action is needed from the editors' end.
So EIP editors decided there is no need to make any steps towards "disclosing a vulnerability" in a finalized EIP. I would like to notify EIP editors that this is a direct violation of security principles.
Here is a description of the vulnerability disclosure practices by Avast, which is a security company.
Vulnerability disclosure is an important process in keeping our products secure. Security researchers find the flaws, report them to the vendors, who then fix them. That's the theory. When it works, it works well; but it doesn't always work.
There are two basic approaches to vulnerability disclosure from the researchers, which are characterized by the terms 'full disclosure' and 'responsible disclosure'. Source: Avast Software
There is another good publication from security researchers that describes a proper process of security vulnerabilities disclosure.
Vulnerability disclosure is very important to make sure that users are safe. If there is a vulnerability in a EIP, and we do have a security problem in ERC-20 that resulted in a loss of $201M already, then it is important to notify all the readers of the EIP that this must be taken into account. Otherwise they will not know about it and they will keep using the EIP with a vulnerability and this will cause even more financial losses for the users.
Refusing to disclose vulnerabilities is heavily criticized by security researchers.
Perhaps this decision was made by EIP editors without any consultation with security experts which could result in such a violation of well-known security principles.
Do somoene of the EIP editors have a security background?
Were security experts involved in the decision making process?
Isn't it reasonable to make security experts decide on how to solve security problems?
SamWilsn
commented
11 months ago There's a bit more nuance to our decision than Pooja included in the summary (which makes sense; it's a summary.)
So first, and most generally, we aren't opposed to vulnerability disclosure. Disclose away, propose better standards, and get implementers to migrate. All good things as far as we're concerned.
The problem is that we don't have a process to deal with vulnerability disclosures, and aren't really equipped to deal with them.
It isn't as simple as just putting a warning on the markdown file. We have a pretty hard rule that Final proposals are immutable. It comes up often, and trust me when I say this, it's better to not reopen that discussion. It boils down to: once contracts are deployed or hardforks forked, it is impossible to change them, so instead of editing the specification they were written against, you write a new and better specification going forward.
Even if we were willing to edit final EIPs, we run into a problem of authority: who determines if a particular disclosure is actually a problem? For example, in this particular case, is sending tokens to a contract a security vulnerability or a user error? Yes, I am aware that blaming the users when the system itself could be designed to prevent it is a common problem in software. I don't want to debate the specifics on this agenda thread, just highlight that these kinds of decisions have to be made by someone. The EIP Editors do not want to be in the position to make decisions like these. We aren't technical experts on every subject, and usually only have a passing involvement in any proposal. I'm going to have to ask you to trust me again, but you really don't want us making these kinds of decisions.
Maybe the answer is that we make a wiki that runs alongside the formal EIP process? I'm not sure. We are willing to work towards a process for publishing vulnerabilities, but we aren't there today.
To answer your specific questions:
- Do somoene of the EIP editors have a security background?
All of us are software developers, including two core developers. I can't say any more specifically than that, since I don't know.
- Were security experts involved in the decision making process?
Not that I'm aware of, no.
- Isn't it reasonable to make security experts decide on how to solve security problems?
Absolutely! We don't want to stand in the way of anyone reporting security issues to implementers.
Therecanbeonlyone1969
commented
11 months ago There's a bit more nuance to our decision than Pooja included in the summary (which makes sense; it's a summary.)
So first, and most generally, we aren't opposed to vulnerability disclosure. Disclose away, propose better standards, and get implementers to migrate. All good things as far as we're concerned.
The problem is that we don't have a process to deal with vulnerability disclosures, and aren't really equipped to deal with them.
It isn't as simple as just putting a warning on the markdown file. We have a pretty hard rule that Final proposals are immutable. It comes up often, and trust me when I say this, it's better to not reopen that discussion. It boils down to: once contracts are deployed or hardforks forked, it is impossible to change them, so instead of editing the specification they were written against, you write a new and better specification going forward.
Even if we were willing to edit final EIPs, we run into a problem of authority: who determines if a particular disclosure is actually a problem? For example, in this particular case, is sending tokens to a contract a security vulnerability or a user error? Yes, I am aware that blaming the users when the system itself could be designed to prevent it is a common problem in software. I don't want to debate the specifics on this agenda thread, just highlight that these kinds of decisions have to be made by someone. The EIP Editors do not want to be in the position to make decisions like these. We aren't technical experts on every subject, and usually only have a passing involvement in any proposal. I'm going to have to ask you to trust me again, but you really don't want us making these kinds of decisions.
Maybe the answer is that we make a wiki that runs alongside the formal EIP process? I'm not sure. We are willing to work towards a process for publishing vulnerabilities, but we aren't there today.
To answer your specific questions:
- Do somoene of the EIP editors have a security background?
All of us are software developers, including two core developers. I can't say any more specifically than that, since I don't know.
- Were security experts involved in the decision making process?
Not that I'm aware of, no.
- Isn't it reasonable to make security experts decide on how to solve security problems?
Absolutely! We don't want to stand in the way of anyone reporting security issues to implementers.
@SamWilsn With standards, there are typically versions see e.g. W3C. Hence, one could create a new version of an EIP. I do not see a process for that referenced in the repo, unless I missed something. It probably would be a good thing to have. Also, the security consideration section would be the place to add any considerations as to potential vulnerabilities, unintended behavior and their mitigation, no? As it is non-normative, it could be updated with a PR, no?
shemnon
commented
11 months ago Apart from EIP editing processes my concern is that there is not any consensus on whether or not this specific claim is a security vulnerability that warrants this kind of action. I see only one or two advocates and no other voices sustaining the vulnerability, and a lot of doubt from those who do look into the issue. Consider also that the most vocal advocate has a competing standard I am concerned that the EIP warning pointing to the standard could be considered advertising (of another standard, not a third party product).
Within the scenarios that Dexaran sets I will concede that EIP-223 is the better standard, but I do not consider those scenarios to rise to the level of a security advisory. I will also advance the idea that some scenarios exist outside of the 223 issues that may come into vogue, such as using ephemeral polymorphic contracts to manage token ownership. What is a security issue in one perspective is a feature from another.
The EIP process does not have the burden of proof to disprove that the claims are worthy of a security advisory. I am of the opinion that instead Dexaran has the burden of proof, and that would involve finding one or more reputable and neutral security researchers whose expressed finding is that a security advisory needs to be added. That is the level of rigor I feel is needed to alter a final standard, even if all that is warranted is to add new security considerations to the security section.
SamWilsn
commented
11 months ago With standards, there are typically versions see e.g. W3C. Hence, one could create a new version of an EIP. I do not see a process for that referenced in the repo, unless I missed something. It probably would be a good thing to have.
The equivalent for EIPs would be to make a new EIP, with a new number. You are correct that there is no in-process way to indicate the canonical successor for a Final EIP, and this is intentional. Conceptually, once an EIP goes to Final it is "owned" by the larger community. Who should have the authority to designate a successor?
Also, the security consideration section would be the place to add any considerations as to potential vulnerabilities, unintended behavior and their mitigation, no? As it is non-normative, it could be updated with a PR, no?
I brought this up as an option on the call (personally I think it's a reasonable modification), and the consensus was that we will not modify Final EIPs to add security considerations.
Dexaran
commented
11 months ago @shemnon
Apart from EIP editing processes my concern is that there is not any consensus on whether or not this specific claim is a security vulnerability
I am a hacker. I have constructed and successfully executed one of the largest consensus-level attacks on EOS mainnet and froze it for almost a month in 2019 with a clearest public record. I know how vulnerability looks like. I can even identify and exploit vulnerabilities that others struggle to see.
I'm saying - this is a security problem and we can call it a vulnerability. This vulnerability can be classified as a "failure of an interface that allows a user to make a security decision". Full description here https://github.com/ethereum/ethereum-org-website/issues/10854#issuecomment-1683873221
I had a nice discussion with OpenZeppelin pointing out that ERC-20 design fits in their "Critical Vulnerability Criteria" according to their bug bounty https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4474
During the discussion there were totally no arguments against the fact that it fits in the critical vulnerability criteria. They just told me "it is well known" and "your submission is incorrect" however this quite confirms that it is not possible to deny it is a "critical vulnerability" according to their bug bounty.
For example this https://kirkpatrickprice.com/blog/secure-coding-best-practices/
And this https://docs.oracle.com/en/operating-systems/oracle-linux/6/security/ol_desprinsc_sec.html
transfer function which means error handling is impossible.transfer and transferFrom function is to allow action (transfer of tokens in this case) to be performed without checking for its correctness.https://gist.github.com/Dexaran/40213a04ce46b394279ac7daa581ce87
So, you have a hacker who has constructed one of the largest consensus-level attacks saying "this is a vulnerability", it perfectly fits in "Critical Security Vulnerability" criteria according to OpenZeppelin bug bounty, it violates well-known standard security practices, it caused a loss of $200M
How do you think is it a vulnerability or at least a security problem?
What consensus are we expected to reach - consensus among solidity devs whose contracts get hacked 2 times out of 3 or consensus among ethereum redditors who don't know how to code? Does it make sense?
Dexaran
commented
11 months ago The EIP process does not have the burden of proof to disprove that the claims are worthy of a security advisory. I am of the opinion that instead Dexaran has the burden of proof, and that would involve finding one or more reputable and neutral security researchers whose expressed finding is that a security advisory needs to be added.
Well, this makes sense but I'm struggling with conflicts of interests for years. For example I can't ask for CertiK's opinion. After they have audited 500 ERC-20 token contracts and assigned a "secure" label on them CertiK will never say "ERC-20 has security problems. For the past 6 years we were assigning 'secure' labels on insecure contracts."
If we take into account that ERC-20 is the most widely adopted component of the ecosystem - there is almost zero chance to find an "independent" security auditor who will not suffer any reputation loss after saying "ERC-20 has security problems".
SamWilsn
commented
11 months ago Consider also that the most vocal advocate has a competing standard I am concerned that the EIP warning pointing to the standard could be considered advertising (of another standard, not a third party product).
Creating another standard and advocating for it is the officially recommended way (as much as anything is official in Ethereum) to deprecate an EIP. Your concern here, however, is why we don't have status: Superseded or superseded-by: 1234 fields that can be modified by an author.
Within the scenarios that Dexaran sets I will concede that EIP-223 is the better standard, but I do not consider those scenarios to rise to the level of a security advisory. I will also advance the idea that some scenarios exist outside of the 223 issues that may come into vogue, such as using ephemeral polymorphic contracts to manage token ownership. What is a security issue in one perspective is a feature from another.
Sure, ERC-223 might be better than ERC-20. Is it better than ERC-1155? Better than ERC-777? Is ERC-223 a worthwhile improvement over ERC-20 considering counterfactual contracts exist? Is the better solution to:
All these questions are subjective. EIP Editors don't like answering subjective questions.
The EIP process does not have the burden of proof to disprove that the claims are worthy of a security advisory. I am of the opinion that instead Dexaran has the burden of proof, and that would involve finding one or more reputable and neutral security researchers whose expressed finding is that a security advisory needs to be added. That is the level of rigor I feel is needed to alter a final standard, even if all that is warranted is to add new security considerations to the security section.
(emphasis mine)
You've just moved where the subjectiveness comes in. Now EIP Editors have to determine who is a reputable and neutral security researcher.
Dexaran
commented
11 months ago @SamWilsn
Is the better solution to: Pick an ERC-165 id that signals ERC-20 token support; and Recommend that wallets:
- Warn when transferring tokens to a contract not supporting that interface id; and
- Warn when transferring tokens to an account that might not be an EOA (i.e. no transactions and no code.)
From security point of view it is not a solution. Again, there are well-known security practices that state "software must be secure by design" and implement "failsafe defaults".
This actually means that any standard that allows a user to make such a mistake (no matter if it can be in theory may be at some point possibly solved by UIs at a higher level which in practice never happens because UI devs introduce new problems rather than solve existing ones) then it is insecure.
This is not my "subjective opinion", this is not something revolutionary, this is not even something new. If someone will dare googling "how to develop software securely" - he will find most of the answers rather quickly and understand that ERC-20 violates very basic things.
You don't need 200 IQ to understand you need to fasten your seat belt when you drive. It's basic safety rule.
All these questions are subjective. EIP Editors don't like answering subjective questions.
If you will follow my links you will find that it's not something subjective or questionable. These are very basic things. If you open any security guideline - you will find them easily.
SamWilsn
commented
11 months ago I guess the best suggestion I can give you, @Dexaran, is: come up with a process for how EIP vulnerabilities should be disclosed and published, then either run it yourself, or ask the EIP Editors to execute it. I fully admit that isn't a satisfying takeaway from this discussion though...
Dexaran
commented
11 months ago @SamWilsn
The problem is that we don't have a process to deal with vulnerability disclosures, and aren't really equipped to deal with them.
I want to start creating one now. What is the best place to start? Create a EIP improvement issue?
It isn't as simple as just putting a warning on the markdown file. We have a pretty hard rule that Final proposals are immutable.
Yea, but we can just put a label on the https://eips.ethereum.org/EIPS/eip-20 without modifying the EIP text at all.
Look, here are EIP labels:
For example you are now placing a big yellow label on draft EIPs saying that "draft EIPs must not be used in production". Can something similar be implemented?
It boils down to: once contracts are deployed or hardforks forked, it is impossible to change them, so instead of editing the specification they were written against, you write a new and better specification going forward.
Unfortunately it doesn't solve problems with existing contracts.
For example if we put a label on ERC-20 saying "it is insecure" people will likely complain to Tether asking them to restandardize USDT or at least do something with it. This can force them to implement a fix. As the result people will not be losing their tokens with new contracts if USDT will be upgraded.
SamWilsn
commented
11 months ago This actually means that any standard that allows a user to make such a mistake (no matter if it can be in theory may be at some point possibly solved by UIs at a higher level which in practice never happens because UI devs introduce new problems rather than solve existing ones) then it is insecure.
With that logic, ERC-223 is fundamentally insecure because you can send tokens to accounts without known private keys or to addresses that will be counterfactually deployed (i.e. CREATE2.) Should ERC-223 get the same warning as ERC-20?
I shouldn't have gone into the weeds in my earlier post; sorry about that :sweat_smile:
This is not my "subjective opinion", this is not something revolutionary, this is not even something new. If someone will dare googling "how to develop software securely" - he will find most of the answers rather quickly and understand that ERC-20 violates very basic things.
I 100% agree that "ERC-20 tokens being sent to address that cannot release them is a problem" is an objective fact. I did suggest adding something to this effect to ERC-20's Security Considerations section on the EIPIP call, but was overruled.
You don't need 200 IQ to understand you need to fasten your seat belt when you drive. It's basic safety rule.
It's impossible to write this without sounding snide, so I ask for your forgiveness, but: you don't need 200 IQ to understand you don't mail cash to a random street address. It's basic financial literacy rule.
All these questions are subjective. EIP Editors don't like answering subjective questions.
If you will follow my links you will find that it's not something subjective or questionable. These are very basic things. If you open any security guideline - you will find them easily.
If we put a warning on every EIP where user error could cause a loss of funds, nearly all ERCs would have it. The subjectiveness is where to draw the line. The amount of money lost is not a good criteria.
SamWilsn
commented
11 months ago I want to start creating one now. What is the best place to start? Create a EIP improvement issue?
For something like this, that's going to get a lot of discussion, I'd probably recommend a thread on https://ethereum-magicians.org/
It's extremely unlikely that we'll adopt a process that requires modifying a Final EIP. The most likely option, in my personal opinion, would be a second domain alongside eips.ethereum.org that we link to, like how discussions-to links already work today. That said, I haven't spoken to the other editors about this, so I can't say anything with certainty.
It isn't as simple as just putting a warning on the markdown file. We have a pretty hard rule that Final proposals are immutable.
Yea, but we can just put a label on the https://eips.ethereum.org/EIPS/eip-20 without modifying the EIP text at all.
Those labels are just generated from the preamble fields.
It boils down to: once contracts are deployed or hardforks forked, it is impossible to change them, so instead of editing the specification they were written against, you write a new and better specification going forward.
Unfortunately it doesn't solve problems with existing contracts.
Agreed.
For example if we put a label on ERC-20 saying "it is insecure" people will likely complain to Tether asking them to restandardize USDT or at least do something with it. This can force them to implement a fix. As the result people will not be losing their tokens with new contracts if USDT will be upgraded.
The Venn diagram of the stakeholders you've mentioned:
Silliness aside, I sincerely doubt that adding a label to ERC-20 will make the slightest difference in adoption. Talking to implementers will.
Therecanbeonlyone1969
commented
11 months ago @Dexaran A standard is not code. At least, it should not be. The code appears in reference implementations of the standard. Reference implementations can be safe or not. And that is the place where security best practices should come into play, not the standard itself.
I agree with you that a standard should point out security, privacy, and internationalization considerations, as applicable, and should be very thorough, IMHO.
@SamWilsn If finals are 100% immutable, then adding a required field in the EIP header such as supersedes would allow a) the original authors to publish a new version of the same standard or b) other authors to publish an improved version of a given standard.
It would then also require a new section(s) for Normative Changes to [EIP] and/ or 'Non-Normative Changes to [EIP]'.
This might be a way to accomplish what @Dexaran tries to accomplish, and keep the Final is 100% final rule in place.
Thoughts?
SamWilsn
commented
11 months ago @Therecanbeonlyone1969
If finals are 100% immutable, then adding a required field in the EIP header such as
supersedeswould allow a) the original authors to publish a new version of the same standard or b) other authors to publish an improved version of a given standard.
I've definitely considered that. supersedes: <earlier eip number> doesn't have the same authority problem as superseded-by: <later eip number>. Aside from some minor rendering concerns, I don't see a supersedes header as problematic.
As you may have noticed, making any changes to the EIP process is... difficult. We're in the process of writing down our governance process (here, with discussions over here). My eventual goal is to provide more flexibility with these kinds of things, and come to decisions more quickly. Maybe supersedes will appear eventually :upside_down_face:
Dexaran
commented
11 months ago @SamWilsn
With that logic, ERC-223 is fundamentally insecure because you can send tokens to accounts without known private keys or to addresses that will be counterfactually deployed (i.e. CREATE2.) Should ERC-223 get the same warning as ERC-20?
I had exactly the same discussion with Nick Johnson 6 years ago and he used the same argument. Here it is: https://www.reddit.com/r/ethereum/comments/6uckdx/comment/dlxw6vu/?utm_source=reddit&utm_medium=web2x&context=3
He intentionally thrown 1.23 ETH to ENS contract and said that "you see, it is also possible". The difference is that in 2017 there were $15,000 lost ERC-20 tokens, today it's $200M at least. And the amount of ETH in the ENS contract barely changed.
Again, there are well-known secure software design principles that state:
Plain ether satisfies these criteria, ERC-223 satisfies these criteria, EOS C++ tokens satisfy these criteria, ERC-721 NFTs satisfy these criteria.
Only ERC-20 does not. And it caused money losses.
you don't need 200 IQ to understand you don't mail cash to a random street address. It's basic financial literacy rule.
"Sending to wrong address" and "misunderstanding internal logic of the smart-contract that can result in unhandled error" are two different problems in my opinion.
The process of exchanging Ether for USDT at Binance looks like this:
The process of exchanging any ERC-20 token or any other asset at Binance looks exactly the same.
The process of exchanging tokens at Uniswap however looks like:
And now we are wandering why there are $200M deposited to contracts directly.
Talking to implementers will.
Talking to implementers in 2017: https://www.reddit.com/r/storj/comments/6ajjo3/comment/dhf95qz/?utm_source=reddit&utm_medium=web2x&context=3
Me: "Your users will lose money if you will use ERC-20 standard"
Implementers: "We know but we prefer to err on the side of well tested code. Let our users lose the money."
Talking to implementers in 2023: https://www.reddit.com/r/ethereum/comments/15lo5x9/comment/jvg4p1c/?utm_source=reddit&utm_medium=web2x&context=3
Me: "Your users will lose money if you will use ERC-20 standard"
Implementers: "We are audited. Our auditors told us we are fine."
Auditors: "Loss of funds is standardized. If we will call it a problem then we will have no clients because we have to call insecure everything that implementers want to create." Big hello to OpenZeppelin as pioneers of this approach https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4451
poojaranjan
commented
11 months ago @Dexaran To be honest, we value your effort in "disclosing a vulnerability" and sharing related sources. But, I agree with Sam that we don't have a process in place for that, yet.
The best way forward is to propose an alternate EIP, take it to the Final status, and educate contract developers about the issues with the existing standard and how the proposed standard is helping with that. Until we have a better standard, people will continue using what is available.
EIP Editors are here to help with the documentation of the standard. If it helps, please follow EIP-5069 where the "Mission" states what EIP Editors do and don't.
To the best of my knowledge, EIP Editors will help authors with whatever guidance is needed to move a proposal to Final. I'll be happy to invite you to PEEPanEIP to share more about the EIP-223 with the community. We welcome your suggestion to create awareness.
SamWilsn
commented
11 months ago @Dexaran
I had exactly the same [...] Only ERC-20 does not. And it caused money losses.
Supposing that:
Then, exactly one of the following is true:
There are a few threads being discussed here, and I want to clarify my position on them:
shemnon
commented
11 months ago The EIP process does not have the burden of proof to disprove that the claims are worthy of a security advisory. I am of the opinion that instead Dexaran has the burden of proof, and that would involve finding one or more reputable and neutral security researchers whose expressed finding is that a security advisory needs to be added. That is the level of rigor I feel is needed to alter a final standard, even if all that is warranted is to add new security considerations to the security section.
(emphasis mine)
You've just moved where the subjectiveness comes in. Now EIP Editors have to determine who is a reputable and neutral security researcher.
(a) The neutral and reputable aspects can be removed, but there needs to be some language that can be pointed at if someone hires "paid shills" (not what's going on here) (b) The main ask here is multiple people with non-uniform stake agree it's an issue. One very insistent reporter does not shift the burden of proof onto the EIP process.
poojaranjan
commented
11 months ago I suppose we should discuss more on EIP related FEM thread. That will help get more eyes on the discussion. Anyway, this issue was created to collect Agenda topics and has to be closed. Closing the issue in favor of #260
SamWilsn
commented
11 months ago @shemnon
(a) The neutral and reputable aspects can be removed, but there needs to be some language that can be pointed at if someone hires "paid shills" (not what's going on here)
So we should reject a security disclosure from an anon, or by the very act of them reporting a vulnerability do they become a security researcher?
(b) The main ask here is multiple people with non-uniform stake agree it's an issue. One very insistent reporter does not shift the burden of proof onto the EIP process.
One very insistent reporter and their sockpuppet accounts can all agree it's an issue. Preventing Sybil attacks is also a subjective thing.
SamWilsn
commented
11 months ago For further discussion: https://ethereum-magicians.org/t/erc-20-vulnerability-disclosures-bears-oh-my/15548
Date and Time
Aug 23, 2023 at 14:00 UTC
Location
Zoom: TBA in the Discord #eip-editing channel
YouTube Live Stream/Recording: https://www.youtube.com/playlist?list=PL4cwHXAawZxpLrRIkDlBjDUUrGgF91pQw
Agenda
1. Discuss Open Issues/PRs, and other topics
Changes to
Finalproposals2. Discussion continued or updates from past meetings
3. EIPs Insight - Monthly EIPs status reporting.
4. EIP Editing Office Hour
5. Review action items from earlier meetings