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?

up vote 33 down vote accepted

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:



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:

  • "The attacker creates a split proposal with the recipient address begin the wallet contract created above." Unable to parse the sentence. It does not make grammatical sense. – Jus12 Jan 20 '17 at 7:20
  • How this is 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. possible ? If gas exceeded shouldn't EVM throw an error no matter what? @BokkyPooBah – alper Dec 29 '17 at 0:26

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

  • Isn't transactions on EVM executed atomically hence race-condition shouldn't have any affect? @Roland Kofler – alper Dec 17 '17 at 8:20

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

  • 3
    If someone is reading this now, just letting you know that Call depth attack is no longer possible after the EIP 150 update. – Honor LT Nov 2 '17 at 9:27

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.