Would `throw` prevent this recursive attack?

Question

I am writing a basic contract to split ethereum evenly between two addresses.

Within my withdraw() function, I have the following code:

function withdraw() {
    uint256 amount = this.balance / 2;
    addressOne.transfer(amount);
    addressTwo.transfer(amount);
}

The problem is, having subsequent transfers like this enables recursive attacks. The owner of addressOne could call the function and overflow the stack right before the other half of the balance is sent to addressTwo.

Executed repeatedly, the owner of addressOne could drain the contract according to the series (assuming an even balance, i.e. no remainder):

My proposed solution then is the following:

if(!addressOne.send(amount)) throw;
if(!addressTwo.send(amount)) throw;

But would throwing like this prevent the same attack?

Answer 1

There are a couple of problems with this contract.

1. Regardless of whether throw reverts the transactions, this wouldn't prevent a re-entrancy attack. If addressOne is the following contract:

   contract AddressOne {
      function() payable {
        OriginalContract.withdraw();
   
        // some case to stop recursion to prevent exhausting gas
      }
   }
   

   addressOne can drain the contract when withdraw is called. You can prevent this by doing one of two things: using a withdraw flow where each address withdraws their own share of the funds as they please or by putting a mutex over the contract global state. If you would like to do the former, check out my splitter-contract for an example.

2. Anyone can call the withdraw function. I don't think this is intended.

Please correct me if I'm incorrect.