Martin Holst Swende @holiman 08:59
The example is a bit contrived, would be intersting to know if thereâs a more common pattern where this could crop up
Eth-Gitter-Bridge @Eth-Gitter-Bridge 09:01
[W Dimitry] Why token transfer logic suddenly depends on sstore gas price?
Alex Beregszaszi @axic 09:01
The brief explanation is that a CALL made to transfer funds, even if no additional gas is specified, will give 2300 gas to the callee. Solidityâs .transfer() function does this and provides no option to the user to override that.
This has not been an issue previously, because even though with 2300 gas one could make a CALL from the callee, there was no way to change a storage entry when SSTORE costs 5000 gas.
After Constantinople the cheapest SSTORE case is 200 gas.
Danny Ryan @djrtwo 09:01
Is it a common pattern to have the security of a transfer premised upon not having enough gas to perform a write?
Martin Holst Swende @holiman 09:01
So a prerequisite is that the attacker managed to get the vulnerable contract to first make the slot in question dirty before the re-entrancy happens
Is it a common pattern to have the security of a transfer premised upon not having enough gas to perform a write?
Iâd say a significant amount of devs operate under that assumption
⌠and later on during a live call on Zoom âŚ
Jason Carver @carver 11:51
I am hearing postponement as the rough consensus on the Zoom call, too. People are starting to evaluate the risks of postponement, and how to mitigate them.
Nick Johnson @Arachnid 11:51
I agree that that should be a showstopper. Not introducing new bugs into previously secure contracts should be right at the top of our priority list.
⌠with some notes from the Zoom call âŚ
Taylor Monahan @tayvano 12:20
Moving information from ZOOM chat to here for posterity
From Yoav Weiss to Everyone: (12:15 PM)
Since weâre delaying the fork and have time to fix it now, Iâd like to re-propose the fix I brought up earlier: Accept the EIP to lower the SSTORE fee, but add a condition that reverts storage access if gasleft < 2300. This preserves the original intention of the EIP (lowering the fees) while preventing exploitation. It is unlikely that a legit transaction will need to access storage while gasleft < 2300.Y
So when we apply the fork again, maybe we could do that and keep it as close as possible to the original fork.
From Nick Johnson to Everyone: (12:18 PM)
Yoav, thatâs an excellent idea for a fix.
Thanks for sharing! Itâs pretty scary how late this came to light but those conversations are mostly reassuring.
I am against postponing, but I think that is mostly because I have always been incredibly salty to anyone that relied on gas stipends for security in the past and this vindicates all of my ranting.
I kind of agree with you about relying on weird behavior for security
A lot of people, me included, originally learned how to write contracts through a combination of the docs, word of mouth and random internet articles, I often heard that these calls were safe against reentrancies. Itâs not easy to split the âweird behaviourâ that shouldnât be relied on from the rest.
Not introducing new bugs into previously secure contracts should be right at the top of our priority list.
This seems like a no brainer to me. However some people in that chat seem to be of the opinion that itâs fine because it âonlyâ punishes contract developers(and their users) that werenât following recommended best practices.
If the above is a path people want to walk down, Iâm going to demand some better resources. Setting the precedent that a previously 100% secure contract can be ruined because the code was not deemed good enough really rubs me the wrong way.
In most articles about solidity you learn that send or transfer guards against reentrancy attacks. However, developers should still incorporate check, effects, interaction of course. In an audit when you see that technically the contract is susceptible to a reentrancy attack I think what often happens is that a note is made: âthe check, effects, interaction pattern is not incorporated, but this does not matter as these type of calls guard against reentrancyâ. The developer now knows that in the future they should watch out for this (when they do an explicit call and not a send or transfer), but there is not an alarm ringing that this should be fixed right now because in a hard fork later on it suddenly can be susceptible to a reentrancy!
Imagine that you are a developer who writes such contract. They are lazy (which is of course not good) and have read the note on this audit but think ânah, this is fine, we wonât change anythingâ. Now the hard fork happens and suddenly the contract is drained because of the hard fork. This will anger a lot of developers and will greatly reduce trust into the Ethereum network.
The main points we need to take away from this is that we need to get much better documentation (also at a low level so we can at least be assured that things who work in a certain way right now also work like this in the future) and that we need to decide what changes of smart contract execution is allowed and what not. Any change in the EVM can change to unsuspected edge cases, such as contracts depending on that extcodesize uses the amount of gas it had before the fork where it was raised to 700 - these types of contracts can suddenly break due to the fork.
<rant>FWIW, I fully sympathize. Such sharp corners / âweird behaviourâ should not leak out to app developers. You shouldnât have to worry about this stuff at all, and itâs entirely possible to design a system that prevents such things from occurring. The tradeoffs of the current system is not safe-by-default, and provides few guard rails. I think that itâs salvageable, and that as a community we could do much more.</rant>
