Return to Revisions

Here is the original blog post that described the recursive calling vulnerability: More Ethereum Attacks: Race-To-Empty is the Real Deal, with suggested remediation for this problem.

From the post:

The Vulnerability

Here is some code; see if you can find the problem.

function getBalance(address user) constant returns(uint) {  
  return userBalances[user];
}

function addToBalance() {  
  userBalances[msg.sender] += msg.amount;
}

function withdrawBalance() {  
  amountToWithdraw = userBalances[msg.sender];
  if (!(msg.sender.call.value(amountToWithdraw)())) { throw; }
  userBalances[msg.sender] = 0;
}

Here's the problem: msg.sender might have a default function that looks like this.

function () {  
 // To be called by a vulnerable contract with a withdraw function.
 // This will double withdraw.

 vulnerableContract v;
 uint times;
 if (times == 0 && attackModeIsOn) {
   times = 1;
   v.withdraw();

  } else { times = 0; }
}

What happens? The call stack looks like this:

   vulnerableContract.withdraw run 1
     attacker default function run 1
       vulnerableContract.withdraw run 2
         attacker default function run 2

Each time, the contract checks that the user's withdrawable balance and sends it out. So, the user will get twice their balance out of the contract.

When the code resolves, the user's balance will be set to 0 however many times the contract was called.

And the suggested remediations, from the post:

Remediation Approach 1: Get Your Ordering Correct

The recommended approach in the soon to be published upgraded solidity examples is to use code like this:

function withdrawBalance() {  
  amountToWithdraw = userBalances[msg.sender];
  userBalances[msg.sender] = 0;
  if (amountToWithdraw > 0) {
    if (!(msg.sender.send(amountToWithdraw))) { throw; }
  }
}

and

Remediation Approach 2: Mutexes

Consider this code instead.

function withdrawBalance() {  
  if ( withdrawMutex[msg.sender] == true) { throw; }
  withdrawMutex[msg.sender] = true;
  amountToWithdraw = userBalances[msg.sender];
  if (amountToWithdraw > 0) {
    if (!(msg.sender.send(amountToWithdraw))) { throw; }
  }
  userBalances[msg.sender] = 0;
  withdrawMutex[msg.sender] = false;
}

Q: When creating smart contracts, DAOs or DAPPs, what measures can I take to ensure I am not vulnerable?

Test, audit, test, audit, ... . As with any software systems, there are many potential areas where bugs can creep in. And the higher value it holds, the more interest attackers will have in it.

The reddit thread Can we please never again put 100m in a contract without formal correctness proofs? suggest some formal correctness proof (, but there can still be bugs).

There will be more advisories coming out in the next few days - I'll update this answer.