Solidity: Language-based fix for "Callstack attack" bug

[taoeffect]

See this work by Andrew Miller and Zikai Wen (don't know their GitHub usernames):

http://hackingdistributed.com/2016/06/16/scanning-live-ethereum-contracts-for-bugs/

The work-around they recommend is highly unintuitive and seems poorly documented, with the bug affecting a "majority of smart contracts":

We start by trying out our heuristics on the Etherscrape repository of Solidity source code. As of March 20, 2016, the Etherscrape repo contained 361 Solidity contract programs, 56 of which contained a send instruction. Of these contract programs, we’d infer that the majority (at least 36 of 56) do not use either of the defensive programming techniques.

And:

Upon inspection, not one of the Solidity programs that passed our heuristic check actually applied the recommended best-practice of testing the callstack directly.

Apparently this bug has caused the loss of thousands of $ worth of ETH already, and therefore seems like something that should receive special attention from the language, perhaps deprecating "send" and creating a new language construct for doing it.

[amiller]

This issue is a good idea. (The post authors are myself and @alexwzk)

Note that the underlying problem is also carried over from Serpent, and it's in part a consequence of peculiarities of the EVM itself. https://github.com/LeastAuthority/ethereum-analyses/blob/master/GasEcon.md#hazards

The Solidity compiler is a great place to put warnings. One option might be to make it an error to use send() but ignore the return code. Or to automatically bracket functions with a "callstack check".

Making send behave exactly like all the other method calls would at least remove the "education surface," and make it easier for programmers to have correct intuitions about its behavior.

It would be a also be a good idea to try to address the "reentrancy" hazard in the same way, perhaps by providing Solidity support for macros, but I guess that should be a different issue.

[chriseth]

We are thinking about deprecating ether and replacing it by a token contract (and then issuing a warning for the use of send). The "reentrancy" hazard is present in every single interaction with other contracts. Programmers need to be aware that any unknown contract could be malicious - interacting with an unknown contract is like reading data from the network.

[amiller]

+1, that's actually a really comprehensive way to prevent this.

Whenever possible, you should only interact with "unknown contracts" indirectly and asynchronously. Sending to a trustworthy external contract is a way to do that.

[taoeffect]

We are thinking about deprecating ether and replacing it by a token contract

Sorry I don't quite understand, could you elaborate on this?

[chriseth]

We could create a token contract (that complies with the standard token API) where you can send ether and receive exactly 1 token per wei. In the same way you can always convert those tokens back into wei.

Transferring tokens and paying with those tokens will be done using the standard token API.

Recommending programmers to use that token contract instead of ether would also automatically open up your contracts to all other tokens that might exist, because they share the same API.

This is also in line with making Ether less special among all currencies/tokens in Ethereum.

[taoeffect]

Cool, thanks for that, @chriseth! That clears things up!

[AFDudley]

The "reentrancy" hazard is present in every single interaction with other contracts. Programmers need to be aware that any unknown contract could be malicious - interacting with an unknown contract is like reading data from the network.

In a world of perfectly rational people this position is correct and reasonable.

But developers on the ethereum blockchain are not perfectly rational people.

You don't only need to empower competent developers you need discourage malicious and incompetent developers

Even in your above quote you're implying that merely "knowing" the contact will provide any smart contract developer with the knowledge of if the contract is malicious or not!

If it's trivial to detect malicious contracts, than it should be addressed in the compiler and run-time. if it's not trivial to detect malicious contracts, maybe the platform should be modified to make such detections trivial. If you believe such detection is impossible or intractable then you are conceding that your platform will never meet reasonable security assumptions and you should publicly advocate your beliefs.

[gcolvin]

It's non-trivial to detect malicious contracts, unless you can apply a specification for what is "malicious" to their bytecode, and the bytecode of all the contracts they call, and so on ...

[robmyers]

Wouldn't using tokens just introduce a layer of indirection, where re-entrancy attacks still apply just with tokens rather than Ether?

Would the language allowing send/call only at the end a public function and the compiler using static analysis to ensure that any path through the code from that function doesn't include any other sends/calls "good enough"?

Or is that naive. :-/

[taoeffect]

Wondering if either of these questions from #662 have relevance to this issue as well?

Q1: Removing recursion seems to me like a simple fix and a small price to pay for addressing this concern.

Q2: I'm no expert on the EVM, but I have this feeling that if we had a language that was based on message-passing (as opposed to function calling), that all of the concerns here might disappear. I.e. like in Erlang or (maybe) Smalltalk. Am I delusional or is there something there to that idea?

EDIT: copy/pasting addendum from #662:

I.e. in a message-passing model, these types of reentrancy bugs should be impossible, I believe.

There is just a queue of messages. That's it. No "function calls".

This is what makes Erlang so stable and is also the model that is used by "latest hotness" elegant & safe languages like Elm.

EDIT2: Also, see Clojure's Agents, a variant of the Actor approach.

[VoR0220]

@robmyers if the token is designed correctly, then no.

[ghost]

@chriseth Is there a clear guide on how to send to other contracts while avoiding the re-entrancy problem?

[chriseth]

@Physes yes, the documentation recommends the withdraw pattern. If that is not feasible, first perform all checks, then all state changes and as a last step, send the ether.

[chriseth]

This has been properly fixed inside the EVM and by a warning for unchecked send in the meantime.