Remediations for EIP-1283 reentrancy bug

hardfork
security
eip-1283

#22

I am not saying we should not decrease costs of operations. I am pointing out that there was complexity in EIP which was not on the plain sight. And it should have been given more weight when discussing the alternatives.

Yes. It has already been proposed by Vitalik at DevCon3, October 2017, in his “Modest Proposal for 2.0”. He suggested to keep Ethereum 1.0 “safe and conservative”. Of course, Ethereum 1x initiative kind of goes opposite to that, but only because we believe that there are SOME changes necessary to keep Ethereum 1.0 alive.


#23

You misunderstand; the EVM itself enforces the gas stipend. If you make a call to another contract that sends value with 0 gas, the recipient contract gets 2300 gas regardless.

That’s the whole point of the EIP - to make the use of temporary storage for cross-contract calls practical. Examples include mutexes and setting temporary token allowances.

It was explicitly designed in as an invariant, because it’s useful to be able to reason straightforwardly about ‘value sends’ while still providing the target contract with the opportunity to execute a little code. I don’t think we should throw that out, even in the (hypothetical and very unlikely) situation that we can without breaking anything already deployed.


#24

This is how I understood it but I wasn’t aware that this was done by the EVM. Is there an issue with removing this gas stipend in the case that send and transfer use the new call context you described for 2.?

I agree that an invariant shouldn’t be changed.


#25

Removing the stipend would break existing code that relies on being able to log an event when it’s sent funds, if it’s called by an existing contract.


#26

I did not know this and assumed you could always forward 0 gas, making transfers to any existing contract impossible.

I see, this is a good use case.

But in that case, I actually think that reducing the stipend is also an interesting option. I misunderstood that the stipend was not the same as the forwarded gas in a call (because solidity forwards this exact amount). Why is this stipend there in the first place? What is the reason for this? Why must we not explicitly forward an amount of gas and why is there a minimum of 2300?


#27

The stipend exists to ensure that a contract that’s sent ether always has the opportunity to log an event to record that fact.


#28

Couldn’t you have it forward as much gas as is required? I’m not understanding why removing this would default it back to 0.


#29

Proper understanding the stipend requires a bit of history.
Stipend to contracts was in EVM from the beginning. But, unfortunately, the case where call value was “0” was forgotten (or rather it was thought that people won’t do it), so there is no stipend in this case. That is why, sending 0 wei to a contract would fail some time in the past, whereas sending non-zero amount would succeed.

Solidity made a work around to fix this - it now generates the code that checks that if the target of the CALL is a contract, and the Call Value is 0, then it adds 2300 to gas given to the call. This part is not in the EVM, it is a patch to fix the usability issue. The more fundamental fix would be to change EVM, but it was never done


#30

This is starting to make a little bit more sense. However, I still do not understand that there always is a minimum of 2300 gas forwarded (and via above text this is apparently the minimum if the endowment > 0). What if I want to transfer Ether but want to explicitly revoke that anything is logged (so gas forwarded is now less than 375). I don’t see why this always has to be minimally 2300.

So this is also the reason why a lot of (older) contracts always send 1 wei with their CALLs to other contracts?


#31

At present, any time you send value with 0 gas, the recipient gets 2300 gas instead. If we removed the stipend from the EVM, in this event the recipient would get 0 gas and immediately fail. This would break a lot of existing code, as well as breaking the current assumption that you can always at least log an event when sent ether.

That’s not a mistake - the intention is just to provide a stipend when necessary to log receiving ether. There’s no reason to log receiving 0 ether.


#32

To be fair, adding new opcodes for transient storage has the exact same issue, just it’s harder to be exploited. Although transient, it’s still a (temporary) state change and can affect things similarly.


#33

I have always been very bothered by the stipend. Not only does it depend on gas prices not changing in order to provide the value that people want it to provide, it also makes it so receiver contracts can’t do interesting things on receipt of ETH when the caller doesn’t provide much gas. There are a number of interesting things one could do in a contract’s default function that are not possible because contract authors (and the EVM and Solidity) keep limiting the gas provided to receivers.

TL;DR: the invariant that people want to maintain is one that I don’t think ever should have existed and should be removed. If not for Ethereum 1.x, then for 2.0.


#34

I disagree here. The transient storage would not be used to store things like token balances. No one would think of using transient storage for that. It could be used to buffer any changes done in the other frames - but it will be amenable to separate analysis, not mixed up with the analysis of the storage.
I take your comment as an agreement that transient storage is safer, because it is harder to exploit, and because it does not implicate pre-existing contracts


#35

Are you sure about this? I just tested 0.4.24, and it makes the call with a gas value of 0.

The idea is that you shouldn’t be able to do that - contracts should always be able to react to being sent ether. Also, why would you want to?

I don’t follow your argument; removing the stipend would make things worse in this regard, not better.


#36

Then removing is the wrong word, 2. would allow for it to be dynamically changed because the re-entrancy danger that the 2300 gas number came from no longer applies.

You can send ether through self-destruct and I don’t believe that sends any gas so the assumption isn’t quite air tight.


#37

Yes, that’s an unavoidable edge-case - not a feature.


#38

Solidity’s address.call.value(x)() does exactly what I think all of the ETH sending mechanisms should do, which is provide all gas (minus stack unwind gas) like it does for any other contract call. I would be fine if CALL with 0 gas either passed 0 gas (and thus failed, because you didn’t use the opcode correctly) or if it passed along all gas like every other contract call does (a reasonable default). My problem lies in the fact that it sends a hard-coded value that is different from what gets sent along with every other contract call and people rely on that amount to prevent certain behaviors that can’t be guaranteed in a world where gas prices change with time (as seen in this EIP).

Note: The above argument is not proposing a migration strategy, only that the pattern people seem keen on keeping (which seems to be the foundation of some arguments here) is a bad pattern IMO and should never have been implemented in the first place and we should not favor a solution just because it retains that pattern.


#39

Right, I understand your objection. But removing the stipend - allowing calls-with-value to have less than 2300 gas - would make the situation worse, since callees could no longer have any guarantees over what they can do when sent value - and would break a lot of existing code.


#40

Can you clarify, what happens if someone issues a CALL with value and specifies 500 gas? Does the EVM increase the gas provided to 2300, or does it make the call with 500 gas? I was under the impression that the bump to 2300 was only if 0 was specified for gas.


#41

You are right, I don’t know where I got from that solidity always forwards 2300 gas. It indeed forwards 0 gas.

I don’t know why you would want this, but I just find this a weird design rationale. This whole stipend makes EIPs harder to review because we now always have to check if you can’t do any fancy tricks with the 2300 - 700 (700 for CALL) gas via reentrancy. I agree with @MicahZoltu that this stipend is dangerous especially if gas prices change. For example in EIP150, what if someone had a contract which logged extcodesize of the called contract at some point and then suddenly could not because this gas was increased (2300 gas was not enough suddenly). I find that this stipend constrains us a lot. That being said we can’t lower this stipend because this will break stuff aswel.

What if we do the same thing for EIP 1283 but now use a new opcode which does exactly the same as SSTORE but also implements EIP 1283 on dirty storage. (This might be an EIP which has been proposed before, sorry if that is the case).