-
Notifications
You must be signed in to change notification settings - Fork 37
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 88 #257
Comments
Can we add "Placing a warning on ERC-20" (from here) on this agenda? |
@SamWilsn added ! |
@Pandapip1 would like to remove the |
@Therecanbeonlyone1969 would like to add NIST as a permitted origin for external links. |
Summary1. Discuss Open Issues/PRs, and other topicshttps://github.com/ethereum-cat-herders/EIPIP/issues/255#issuecomment-1671818828
ethereum/EIPs#7478
ethereum/EIPs#7388
ethereum/EIPs#7346
ethereum/EIPs#5533
add NIST as a permitted origin for external links
https://github.com/ethereum-cat-herders/EIPIP/issues/257#issuecomment-1688478926ethereum/EIPs#7468 Ref Discord
Changes to Final proposalsethereum/EIPs#7506
ethereum/EIPs#7505
ethereum/EIPs#6860
2. Discussion continued or updates from past meetingsEIP-ERC GitHub repositories |
@poojaranjan @SamWilsn @gcolvin
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.
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.
|
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:
All of us are software developers, including two core developers. I can't say any more specifically than that, since I don't know.
Not that I'm aware of, no.
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? |
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. |
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?
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. |
1. I'm a hacker and I am saying it is a vulnerabilityI 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 ethereum/ethereum-org-website#10854 (comment) 2. OpenZeppelin "confirms" it is a vulnerabilityI had a nice discussion with OpenZeppelin pointing out that ERC-20 design fits in their "Critical Vulnerability Criteria" according to their bug bounty OpenZeppelin/openzeppelin-contracts#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. 3. There are well-known secure software development practices and ERC-20 design violates 3 of themFor 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
4. It resulted in a loss of $200Mhttps://gist.github.com/Dexaran/40213a04ce46b394279ac7daa581ce87 ConclusionSo, 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? |
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". |
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
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.
(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. |
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.
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. |
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... |
I want to start creating one now.
Yea, but we can just put a label on the https://eips.ethereum.org/EIPS/eip-20 without modifying the EIP text at all. 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?
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. |
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. I shouldn't have gone into the weeds in my earlier post; sorry about that 😅
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.
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.
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. |
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
Those labels are just generated from the preamble fields.
Agreed.
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. |
@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 It would then also require a new section(s) for This might be a way to accomplish what @Dexaran tries to accomplish, and keep the Final is 100% final rule in place. Thoughts? |
I've definitely considered that. 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 |
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.
"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 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 OpenZeppelin/openzeppelin-contracts#4451 |
@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 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 |
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:
|
(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) |
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. |
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?
One very insistent reporter and their sockpuppet accounts can all agree it's an issue. Preventing Sybil attacks is also a subjective thing. |
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
requires
header ethereum/EIPs#7478Changes to
Final
proposals2. 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
The text was updated successfully, but these errors were encountered: