Data Gathering for Post-Mortem of Constantinople Hard Fork Delay
Opened this issue · 15 comments
This thread is designed to capture information, thoughts, ideas for topics to discuss in a post-mortem of the decision to delay the Constantinople hard fork, which are unable to be addressed in this moment.
Thank you for your contributions and patience.
Q: Why was the possibility of reentrancy discovered so late?
A: Via gitter, by @holgerd77:
(...) I think one part of the story might be the not-so-long-yet availability of
Constantinopledev tools. We released theEthereumJSConstantinople VM 22 Nov 2018, it took Truffle some time to integrate and the firstConstantinople-ready Ganache version came out just 6 days ago. Since ChainSecure uses this first beta to test the vulnerability my assumption is that these releases might have triggered experimentation, will investigate this further.One outcome of this might be to take dev tool readyness stronger into account when planning a hardfork date, and make release of the 2-3 most used tools (and not just the VM as some base layer) a precondition for some date settlement.
These are, obviously, "candidate Q" and "candidate A".
Tip: to get markdown if you don't have Owner access to this repo, go to the gitter message, click the message menu icon ..., and select "Quote".
Some bullet points I've been collecting:
Things that could / should be considered when writing / choosing / merging EIPs:
- Modifying opcodes / the EVM can have unintended consequences on already-deployed contracts
- Changes of these types need to be made exceedingly carefully
- Smart contract developer assumptions and patterns being used by developers should also be analyzed when reviewing EIPs
- Detecting unintended consequences of updates require imagination / research across the entire stack, not just technical reviews or audits
- How should EIP’s be analyzed? Is excessive community discussion / time enough? External audits? In this case, this EIP was open for a long time and had a lot of eyes on it.
Potentially things clients should do:
- Ethereum clients should have a way for node operators to simply run with specific flags so creating and deploying releases isn’t necessary in situations like this
Misc
- Consider worst-case scenarios when choosing fork names. "ConstantiNOPEl" was too damn easy. 😅
I made this during the emergency call, "The Decision Process For The Constantipole Ethereum Hard Fork / Network Upgrade Postponement":
https://docs.google.com/document/d/1gr0hHAxobzpLYDXZnZFk6qPuv8_to3XLNQ9TvXMCWxw/edit?usp=sharing
I added ideas and suggest we all deconstruct the process and suggest ideas for future decision making ✍️
Secondly, on how this slipped through for so long. Is there any way automated testing can be improved to catch this sort of thing or is it something that needs to be manually discovered? Any lessons learned?
All of the really nasty security issues that we had have been around the interactions between different components. The quadratic DoS attacks combined EVM memory and the call stack frame or reverts and the call stack frame, this potential threat arose because of interactions between the default gas in send, SSTORE gas costs and re-entrancy issues. So if you have N protocol features, there are N2 ways they could potentially break.
I would say my personal takeaway from this is to be much more explicit about writing down invariants (properties guaranteed by the protocol) that we rely on so we can check against them when changing things.
Q: Why was the possibility of reentrancy discovered so late?
A: Via gitter, by @holgerd77:
(...) I think one part of the story might be the not-so-long-yet availability of
Constantinopledev tools. We released theEthereumJSConstantinople VM 22 Nov 2018, it took Truffle some time to integrate and the firstConstantinople-ready Ganache version came out just 6 days ago. Since ChainSecure uses this first beta to test the vulnerability my assumption is that these releases might have triggered experimentation, will investigate this further.
One outcome of this might be to take dev tool readyness stronger into account when planning a hardfork date, and make release of the 2-3 most used tools (and not just the VM as some base layer) a precondition for some date settlement.These are, obviously, "candidate Q" and "candidate A".
Tip: to get markdown if you don't have
Owneraccess to this repo, go to the gitter message, click the message menu icon..., and select "Quote".
Adding my reply from gitter:
I share the concern (I'm a member of the Truffle team and oversee the development of ganache-core and ganache-cli).
Who should I be coordinating with to make sure we're supporting things appropriately here?
also if there's a formal (or informal) retrospective on this, I'd be happy to participate
Regarding automated testing: this time, it was not discovered by fuzzing.
Regarding automated testing: this time, it was not discovered by fuzzing.
Is this the kind of thing that fuzzing is looking for? The problem is that an invariant is being broken which nobody had previously written down.
Had some exchange on this with Ralph who is the organizer of our local Vienna Ethereum meetup and helped to discover this: he told me that he actually knew about this for months (sigh) and just thought that ChainSecurity would have already reported this.
Instead of blaming anyone here (Ralph: surely not, ChainSecurity: there might be some questions) I think we can go one level higher and state that there are definitely some communication issues here, since the information was there in the community, it was just not possible to harness.
Ideas to address this could be some bug bounty program, or at least some "Community call to submit every suspicion of a bug/fork inconsistency/weird behavior" blog post somewhere in a fork preparation process.
I wouldn't say "knew", more like "suspected". I thought surely somebody else already thought of this at some point (since it seemed obvious) and that surely it wouldn't work for some reason. Anyway I wanted to test this on Ropsten first (as I already submitted a bounty in the past and back then it turned out to be an issue with testrpc instead), but that was a side project that just ended up getting delayed and delayed again for various reasons.
It was only during a recent discussion with chainsecurity that I realized this really might be a serious problem. That being said it might have been better to already report this immediately instead of trying to scan for vulnerable contracts first.
That being said it might have been better to already report this immediately instead of trying to scan for vulnerable contracts first.
This cannot be emphasized enough. While I understand the desire to fully verify a problem/bug before reporting, doing something is better than nothing. That something could be bringing it to the attention of developers, other security researchers, or a variety of other people in this space. Another option would be to post on the EIP itself.
What types of things could be done to help ensure people like Ralph put the information out there, even if its not a confirmed bug? Brainstorming here....
- Ensure there are multiple ways to contact relevant folks ranging from serious to casual
- Could cover the full range of bug bounties, relevant chatrooms, encourage people to comment on EIPs, point people to Github issues, security@, bugs@, PGP, support email, forums, etc.
- This way reports aren't limited to those who can utilize PGP or want to create a PoC and there are more casual ways to bring something to attention than sounding all the alarms.
- Perhaps a simple blog post or page on ethereum.org/security various ways of getting in touch with people who can do something about it.
- I personally think the security Telegram channel is an excellent place full of heaps of relevant security people (not just "eth core devs") who are willing to dive deeper or help confirm/eliminate concerns.
Ideas to address this could be some bug bounty program, or at least some "Community call to submit every suspicion of a bug/fork inconsistency/weird behavior" blog post somewhere in a fork preparation process.
Both the Ethereum Foundation and Parity Technologies have a bug bounty program. I'm rather surprised this is not known.
I'm guessing there's some diffusion of responsibility at play here.
Another suggestion: a hardfork drinking game bingo card should be made for all upcoming forks. Hudson has an example card template that can be used.