It's been a tough week for Ethereum. With TheDAO drama ongoing, Ethereum Core announced a wallet vulnerability a few days ago.

The writing was measured, although honest. But it wasn't easy for app developers to understand the attack vector. Let's dig in.

As always, I'm available for security contracting and smart contract development: check out the contact section of my site.

Who's Calling? Internal Edition

Solidity has a standard way to check who's calling a function - msg.sender - and is used everywhere. msg.sender is guaranteed to be the address of the contract or public key of the caller of the first function to be called inside the current contract scope.

That sounds more complicated than it is - let's get a little chain of calls going to see what's what.

function checkOwner() internal { if (!(msg.sender == owner)) { throw; } } function spendMoney(address destination, uint amount) { checkOwner(); if (!destination.send(amount)) { LogFailedSend(destination, amount) } }

We've written some reasonably secure code here -- the only public access point is spendMoney. The first thing it does is make sure that the "owner" of the contract is calling. Once it's certain that the caller is the owner, the function tries a send, and if it fails, logs the failure.

Notice that checkOwner is called by the spendMoney function. Nevertheless, its msg.sender value is set to the original caller of the function, not the spendMoney function's contract address.

Who's Calling: External Calls

Let's look at a different plan for calling: we're going to call the user's wallet on their behalf. The UserWallet contract is a safe wallet, and our contract is going to trigger the spend.

contract UserWallet { function checkOwner() internal { if (!(msg.sender == owner)) { throw; } } function doSpend(address destination, uint amount) { checkOwner(); if (!destination.send(amount)) { LogFailedSend(destination, amount) } } contract OurContract { function spendMoney(UserWallet w, address destination, uint amount) { w.doSpend(destination, amount); } }

What is the value of msg.sender in the UserWallet.checkOwner() call? It is the address of the OurContract contract -- that address becomes the msg.sender , replacing the original caller's address, as soon as an external call is made.

This makes a lot of sense. Otherwise, we can imagine any number of terrible, terrible attacking wallets that could pretend to be worth sending money to. As soon as someone was stupid enough to send them money, they could execute a remote attack on the sender. In fact, if msg.sender worked any other way, there would be horrible security bugs.

Imagine msg.sender told contracts who the original caller was, the one who started it all. Then this attack contract would drain all the money out of every sender wallet (pending gas limits, a topic for another day).

contract UserWallet { function send(address dest, uint amount) { if (msg.sender != owner) { throw; } dest.send(amount); } } contract AttackWallet { function() { UserWallet w = UserWallet(userWalletAddress); w.send(thiefStorageAddress, msg.sender.balance); } }

Note My original code had a logic error, helpfully corrected by redditor crypto-jesus. His modifications are included above.

Even though the UserWallet is checking the sender, if msg.sender worked any other way, this attack would work. The only way this attack won't work is if msg.sender is updated to the calling contract's address on each external call. Clearly not doing this is not a viable infrastructure.

Who's Calling, Developer Edition

There is a way to check the "original caller" information. It's called tx.origin . tx.origin walks all the way up the call stack and tells you who originated your call.

When I say it that way, I bet if you're a developer you think "that sounds useful!" But, tx.origin is exactly what we just decided is a terrible, terrible thing that shouldn't get anywhere near user authentication.

What Happened?

So, what happened in these older wallets? They used tx.origin to test for who was calling, rather than msg.sender . That's the total attack surface of the bug.

Now we can decode these statements from the security post:

The attack can happen if an affected wallet interacts with a malicious contract

OK, we can imagine that contract -- it will have the right to call any wallet function that mistakenly used tx.origin to check permissions because the owner of the wallet will have initiated the action.

OR if the owner account of an affected wallet interacts with a malicious contract that knows the address of his wallet.

And we can imagine that as well, and this is horrible, a callstack could be created like this:

owner of vulnerable wallet -> appealing attacker attacker contract -> separate wallet contract wallet contract gets called, and owner shows in tx.origin.

I'd like to imagine this is not out in the wild. It would be very confusing to be going about your business gambling at some appealing new casino or sending money to a new and interesting DAO only to see that your (you thought) unrelated wallet contracts got drained at that very moment.

Vulnerability Assessment