What is a recursive calling vulnerability?

Question

What is a recursive calling vulnerability exactly?

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

Answer 1

A Simpler Explanation

1. The attacker creates a wallet contract (0xc0ee9db1a9e07ca63e4ff0d5fb6f86bf68d47b89 in the 17/06/2016 attack) with a default (or fallback) function () to call The DAO's splitDAO(...) function a number of times. Following is a simple default function ():

   function () {
      // Note that the following statement can only be called recursively
      // a limited number of times to prevent running out of gas or
      // exceeding the call stack
      call TheDAO.splitDAO(...)
   }
   

2. The attacker creates (or joins) a split proposal (#59 in the 17/06/2016 attack) with the recipient address begin set to the wallet contract created above.

3. The attacker votes Yes on the split proposal.

4. After the split proposal expires, the attacker calls The DAO's splitDAO(...) function.

   a. The splitDAO(...) function calls the wallet contract's default function () as part of sending the ethers to the recipient.

   b. The wallet contract's default function () calls The DAO's splitDAO(...) again, which repeats the cycle from a. above.

   c. The wallet contract's default function () must ensure that an error is not thrown as the transactions will be rolled back if the call stack or gas is exceeded.

Following are snippets of The DAO's source code involved in this type of attack:

DAO.splitDAO(...):

The problem in the following code is that the payment is made (statement withdrawRewardFor(msg.sender);) before resetting the variables that keep track of the payments the recepient is entitled to receive (balances[msg.sender] = 0; and paidOut[msg.sender] = 0;).

    function splitDAO(
        uint _proposalID,
        address _newCurator
    ) noEther onlyTokenholders returns (bool _success) {
        ...     
        withdrawRewardFor(msg.sender); // be nice, and get his rewards
        totalSupply -= balances[msg.sender];
        balances[msg.sender] = 0;
        paidOut[msg.sender] = 0;
        return true;
    }

DAO.withdrawRewardFor(...):

    function withdrawRewardFor(address _account) noEther internal returns (bool _success) {
        if ((balanceOf(_account) * rewardAccount.accumulatedInput()) / totalSupply < paidOut[_account])
            throw;

        uint reward =
            (balanceOf(_account) * rewardAccount.accumulatedInput()) / totalSupply - paidOut[_account];
        if (!rewardAccount.payOut(_account, reward))
            throw;
        paidOut[_account] += reward;
        return true;
    }

ManagedAccount.payOut(...):

The statement _recipient.call.value(_amount)() sends the ethers to the recipient's account, in this case the wallet contract's default function () is called which enables the DAO.splitDAO(...) function to be called recursively.

    function payOut(address _recipient, uint _amount) returns (bool) {
        if (msg.sender != owner || msg.value > 0 || (payOwnerOnly && _recipient != owner))
            throw;
        if (_recipient.call.value(_amount)()) {
            PayOut(_recipient, _amount);
            return true;
        } else {
            return false;
        }
    }        

See also:

• Chasing the DAO Attacker’s Wake - "you cannot assume anything about the state of your contract after the external call is executed", and "do not call external contract code in your contract using Solidity’s call construct, ever if you can avoid it. If you can’t, do it last and understand that you lose all guarantees as to the program flow of your contract at that point".
• Analysis of the DAO exploit - this link provide a good analysis of the attack, including the method the attacker managed to repeatedly call splitDAO(...) by transferring away their The DAO tokens before their token balance is set to 0 and then transferring it back to repeat the attack. (Thanks @eth for the answer in What was the second vulnerability used in The DAO attack on 17 June 2016?).
• The big theDAO heist FAQ
• Deconstructing theDAO Attack: A Brief Code Tour
• Which accounts are involved in mounting the recursive call vulnerability attacks on The DAO?
• Which split proposal was used to mount the recursive call vulnerability attack on The DAO?
• How was the recursive call vulnerability attack conducted via proposal #59 when there were no Yes votes?
• Is there any way to determine how long it took for the DAO attacker to deploy the attack?
• Can I execute a contract function by sending a regular transaction to the contract?
• How much computation can be done in a fallback function?

---

More Background Information

Here is the original blog post from Peter Vessenes that described the recursive calling vulnerability in DAOs : 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;
}

---

And from the post by user eththrowa in The DAO forum post Bug discovered in MKR token contract also affects theDAO - would allow users to steal rewards from theDAO by calling recursively:

This bug: https://www.reddit.com/r/ethereum/comments/4nmohu/from_the_maker_dao_slack_today_we_discovered_a/57 Is also present in theDAO code - specifically here in the withdrawRewardFor function DAO.sol:

if (!rewardAccount.payOut(_account, reward))
   throw;
paidOut[_account] += reward;
return true;

and here in managedAccount.sol

function payOut(address _recipient, uint _amount) returns (bool) {
        if (msg.sender != owner || msg.value > 0 || (payOwnerOnly && _recipient != owner))
            throw;
        if (_recipient.call.value(_amount)()) {
            PayOut(_recipient, _amount);
            return true;
        } else {
            return false;
        }
    }

This would allow a user to drain many times his entitlement by calling the contract recursively. Oddly enough the slockit team spotted this bug here in the proposal section:

// we are setting this here before the CALL() value transfer to
// assure that in the case of a malicious recipient contract trying
// to call executeProposal() recursively money can't be transferred
// multiple times out of the DAO
p.proposalPassed = true;

but missed it in the reward section.Obviously there are not yet any rewards in theDAO so this is not an issue that could cost money today.

---

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.

From the Ethereum blog CRITICAL UPDATE Re: DAO Vulnerability:

Contract authors should take care to (1) be very careful about recursive call bugs, and listen to advice from the Ethereum contract programming community that will likely be forthcoming in the next week on mitigating such bugs, and (2) avoid creating contracts that contain more than ~$10m worth of value, with the exception of sub-token contracts and other systems whose value is itself defined by social consensus outside of the Ethereum platform, and which can be easily “hard forked” via community consensus if a bug emerges (eg. MKR), at least until the community gains more experience with bug mitigation and/or better tools are developed.

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 weeks - I'll update this answer.

Some resources:

• Thinking About Smart Contract Security
• Smart Contract Security
• Scanning Live Ethereum Contracts for the "Unchecked-Send" Bug (from @Roland Kofler's answer in How did “the DAO” attack happen on 17 June 2016?).
• Thoughts on The DAO Hack (also from @Roland Kofler's answer in How did “the DAO” attack happen on 17 June 2016?).

Answer 2

If your code looks like this in pseudo code:

function do:
   if (pool has mymoney = true)
     split(mymoney) 
     pool has mymoney = false

By repeatedly calling that function, you have sort of a race condition where you are allowed to spend your money twice, trice, ... ad infitum.

Fix is simple, reverse two operations:

function do:
   if (pool has mymoney = true)
     pool= pool - mymoney // 2
     split(mymoney) //1

See this commit for example of the fix

Answer 3

A "recursive calling vulnerability" is an ambiguous term that should be avoided because it is imprecise and can mean 2 things.

Reentrant attack

You probably mean "reentrancy vulnerability" or "reentrant attack", which is what @Roland's answer describes. Note: not all reentrant attacks have to be recursive (in the sense that malicious code does not have to reenter the same way: it can reenter a contract via any externally accessible function).

http://forum.ethereum.org/discussion/1317/reentrant-contracts

https://github.com/LeastAuthority/ethereum-analyses/blob/master/GasEcon.md

Call depth attack (no longer possible with EIP 150)

In Ethereum, a "call depth attack" is also possible (one of the ways it can be performed is with recursive calls).

How does the stack depth attack make a send() silently fail?

Callstack attack