My two cents: if the attack is verified then the hard fork should be delayed. Many developers have assumed in the past that
transfer and send are safe against re-entrancies. It is just time until contracts are found which can be attacked. This is not fair and will reduce trust in the Ethereum network. If it is implied (in many documents) that these functions are re-entrancy safe and that suddenly these are unsafe against these attacks this will reduce trust in the network a lot since the way how things work apparently can change over time.
^ this. Never mind custom made multisigs by Bob or Alice, if things like Compound.Finance or MakerDAO's CDPs suffer because of this, these enormously successful and growing DeFi projects will be left with no choice than to say "Okay, guess the logic of the program can change, we can't make it work here, let's leave". (illustrative example, those are probably safe). We definitely don't want another DAO or several.
On the call there is talks of using a collaborative sheet for analysis of smart contracts so people aren't duplicating work. I’m not 100% of what the column headings should be but here’s the sheet that we can all use (feel free to be opinionated on what the headings should be): https://docs.google.com/spreadsheets/d/1GS98k8YosBsqV1UVq57vX-PTOUAcCCcRjJ847H8Y4hw/edit?usp=sharing
This is the test from the ChainSecurity truffle
@hayesgm I agree that even with transfer and send people should not have re-entrancy attacks available even if that doesnt work due to gas. But, I think most people have always had in the back in their heads that it is never possible to change storage using transfer or send which is hence now possible
I guess it's not even limited to one slot but actually multiple since every storage slot write which is dirty is 200 gas
I am pretty sure that if we check etherscan we can find some contracts which hold a small amount of eth which are now suddenly drainable due to the hard fork
It's definitely something people use at least as a safety net, though, if not more than that.
I strongly suggest that the fork is delayed because this would honestly raise a lot of confusion in the development community. If it is strongly implied some things are not possible but due to a hard fork it is suddenly possible that is really discomforting and reduces a lot of trust. Still want to mention the Core Dev team has done an amazing job but I also want to thank ChainSecurity for raising this point now (although its really short notice) - there now is some time to fix this.
@homotopycolimit it confirms that a re-entrancy attack is possible on constantinople using transfer/send
Something like this? https://etherscan.io/address/0x48e6ca91001b3ff03ec6bdb42f40179644ff827a#code
This would be a great use for Certora (http://www.certora.com/). The team hooked up an SMT-solver which can easily run a test over EVM byte-code and verify a property (such as a check for these unsafe transfers under Constantinople). Given a few days, we could automatically verify a mass of contracts and authoritatively decide if each is vulnerable or not.
https://etherscan.io/address/0xf4a3679eb0a3d9e8af9824a29bd32dd98d1e7127#code this looks attackable too (but might use too much storage slots)
The sell price is updated after transfering. So you can sell it twice for the same price
And might even have unintended side effects
Look I'm not really checking this but I'm mainly pointing out that there are many OTHER effects which are also possible here
Contracts might not be drainable but there suddenly might be problems updating prices or things, or hence small errors in prices
Main point is, what if due to general consensus like "transfer/send is ok against re-entrancy" you have a non-drainable contract. Now with the fork it suddenly is drainable. This makes Ethereum much less decentralized because you now have to trust that changes in those semantics are not going to hurt contracts which you deployed.
Main point is, what if due to general consensus like "transfer/send is ok against re-entrancy" you have a non-drainable contract. Now with the fork it suddenly is drainable. This makes Ethereum much less decentralized because you now have to trust that changes in those semantics are not going to hurt contracts which you deployed.
