So, I'll admit, when something fails as spectacularly as the DAO did, I can't help but watch. And then, after the dust has settled, I can't help but dive in and find out just what went wrong. My previous post on Proof-of-Work and consensus was along the same lines: how was it possible for non-voting majority to lose to a strongly-motivated, voting minority? It raised some serious concerns (to me, anyway) about the security of PoW, especially as it pertains to the economic incentives that are supposed to add security to the network.
After having successfully replicated the DAO attacker's attack, as well as an unrelated attack used to lock down splits, I decided to go a thorough documentation pass over the DAO's code, to see how these attacks fit together. In the process, I discovered a few more, as well as some very poor design decisions in other parts of the code.
As it turns out, the DAO's code is written in such a way to allow for someone to exploit it in nearly every case: the "failed to meet funding" case, the "received rewards/DAOrewards" case, and finally, the split case. In fact, the only process in the DAO which could be said to operate smoothly and without exploit (as far as I can tell) is the execution of a proposal.
This is... interesting, to say the least. So, let's break down all these failure modes, and look at the associated code.
reward and DAOReward exploitFirst, let's start with the ManagedAccount contract. This is supposed to create an account that can be utilized by only a particular owner, unless, of course, a flag has been set that unrestricts access.
Given that this is used for the extraBalance, rewardAccount, and DAOrewardAccount subcontracts, it's a little interesting that two out of the three would be configured for unrestricted access.
The most glaring problem here, of course, is the payOut method.
We'll drop that here (with my annotations) for easier reference:
// > Alright, here's where things get a little weird. This is a method
// > for transferring value from this account to a specific recipient.
// > However, it doesn't really have much in the way of protections against
// > unauthorized people calling it.
// > In fact, both the DAORewardAccount and rewardAccount `ManagedAccounts`
// > deployed by the DAO specifically disable the "payOwnerOnly" flag on creation.
// > This means anyone can call this function, with any amount, and any recipient.
// > This is a pretty glaring security hole.
// > Furthermore... there's the use of `call` over `send`. The point of this
// > entire function is to simply transfer value from this account to a specified
// > recipient. There's no need to `call` the recipient when a `send` will suffice.
// > So not only do you have an unrestricted method for moving value from this account,
// > you also have one that's intentionally invoking remote code.
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;
}
}
As noted, this function does a couple very suspect things.
First, it uses .call when .send would be more appropriate. The idea is for it to move funds to a target account, and this is exactly what .send is supposed to do. Using .call here is quite interesting, especially since the authors later prove they know how and when to use .send, and when using .call is actually appropriate.
Second, if payOwnerOnly is set to false, anyone can use this function. This means, if there are funds in either the rewardAccount or DAOrewardAccount, anyone can move them anywhere. This is a huge security hole that anyone reading the code should have been able to catch.
Which, of course, just goes to show I never read the code all the way through, before.
It's worth noting here that the former of these two flaws is something you'll want to keep in mind for later.
Another interesting takeaway from this particular exploit is how the extraBalance account doesn't have this flaw--because it's properly restricted to allow only the owner to move funds. If that's the proper mode for this account, it begs the question: why make that a configurable flag at all?
!isFueled exploitRemember how I mentioned that the DAO could be exploited in the event that it failed to reach its funding goal? That even the failure mode of the funding phase in itself had failure modes?
Let's take a look at the TokenCreation contract's refund method
// > Here's another interesting function. This never came to be,
// > but if the DAO didn't meet its funding goal, this would have also
// > been vulnerable to re-entrancy. This, again, is due to the use
// > of `.call` when `.send` would be the appropriate method to use.
// > Like the split recursion attack, the balances of the sender
// > aren't cleared until after moving the balances.
function refund() noEther {
if (now > closingTime && !isFueled) {
// Get extraBalance - will only succeed when called for the first time
if (extraBalance.balance >= extraBalance.accumulatedInput())
extraBalance.payOut(address(this), extraBalance.accumulatedInput());
// Execute refund
if (msg.sender.call.value(weiGiven[msg.sender])()) {
Refund(msg.sender, weiGiven[msg.sender]);
totalSupply -= balances[msg.sender];
balances[msg.sender] = 0;
weiGiven[msg.sender] = 0;
}
}
}
Now, the attack is outlined in my annotations, but we'll just break it down here.
This function only works if we pass the closing time of the funding phase, but don't meet the funding goal (meeting the funding goal is what causes isFueled to be set). If we're past the closing time and we're not funded, this function moves the contents of the extraBalance account back to the account that implements this function (the DAO), and then sends the sender his contribution back. Then it proceeds to zero out his balances, both of his Ether contribution, and his acquired tokens.
It also uses .call to send the Ether, rather than .send.
It's worth noting here that even if it used send, it would still be vulnerable to draining. There's no failure mode if the .call (or the theoretical .send) fails... In fact, all a contract would need to do is return false from its fallback function to to prevent its balances and tokens from being reset. It could then call the refund function again and again until the balance was drained.
The use of .call only exacerbates this, since it allows for this to be done multiple times per transaction, through re-entrancy.
So where does this leave us? Well, the "acquired rewards" case is exploitable, and now the "failed to fund" case is exploitable. Let's see what else is exploitable.
splitDAO exploit.Everyone knows this one, so I'll just gloss over it. Basically, calling splitDAO calls withdrawRewardFor, which executes the aforementioned vulnerable payOut method, allowing re-entrancy and further providing the opportunity for an attack to transfer his tokens away before returning up the stack, so his tokens can also be re-used.
However, I should stop here, and point out something about withdrawRewardFor:
// > This one, again, is vulnerable to re-entrancy, because it calls the insecure
// > `payOut` method. In fact, the above method can be used with a token transfer
// > to enable an interesting token-tainting attack, that compounds the `paidOut`
// > that gets transfered with tokens as they move from owner to owner.
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;
}
It's interesting to note that this method can actually fail. It can fail in the event that the account we're withdrawing on behalf of has already been paid out some rewards, and when the rewardAccount lacks the funds to cover... what they've already been paid out?
Here's where things start really getting interesting. This function fails when the tokenholder's fraction of the total accumulated rewards is less than what they've already been paid. This seems like a strange condition on the surface--however, the next line indicates why this is being tested. This is to prevent integer rollover issues when subtracting out what's already been paid from what's owed.
It still raises and interesting question, though... how and why could this fail? Token rewards are always paid out on the basis of their fractional proportion of the total of all rewards ever, and are transferred as fractions of the account holder's total token holdings. This means if I've been paid out 1000 ether and hold 100 tokens, if I give you 90 of them, you'll be considered to have been "paid out" 900 ether and I'll remain with 100. But my 1000 ether was just my 100 tokens' fraction of the total amount of tokens issued, meaning if 10,000 were issued, the total reward accumulated must be 100,000 ether.
Makes sense.
Except when you remember that payOut is broken. Which brings us to our next attack:
I was tipped off to this one by the actions of (what later was identified as) the "Robin Hood Group", just prior to their attempt to extract the ether mistakenly (?) deposited into the DAO after the fork.
This one is particularly clever, as it leverages all the above exploits. Let's focus on the transfer function, as overridden by the DAO itself:
// > Here's the other piece to the "token tainting" attack.
// > Since the `paidOut` values are transferred in proportion to
// > the tokens being moved, it means the fractional amount of `paidOut`
// > moves with tokens forever. It can be diluted by mixing high-penalty
// > and no-penalty tokens, but the `paidOut` penalty can only be removed by
// > splitting from the DAO.
// > It's worth noting that `paidOut` is only really used by `withdrawRewardFor`
// > and is the only reason that function can fail.
// > Interestingly, `withdrawRewardFor` is an integral part of `splitDAO`.
// > Huh. There's that function again.
function transferPaidOut(
address _from,
address _to,
uint256 _value
) internal returns (bool success) {
uint transferPaidOut = paidOut[_from] * _value / balanceOf(_from);
if (transferPaidOut > paidOut[_from])
throw;
paidOut[_from] -= transferPaidOut;
paidOut[_to] += transferPaidOut;
return true;
}
This attack works by combining withdrawRewardFor, the open access to the rewardAccount, and the transfer function defined above. It works roughly like this:
parent and child) to fully extract that amount of Ether from the reward account. This requires a bit of math, but it's only algebra, and not terribly hard. Either the parent or the child should accumulate some paidOut before starting this attack.parent contains enough tokens to execute this attack, send the chosen amount of Ether to the rewardAccount, and then call the exploit function on child.child calls getMyReward on the DAO.rewardAccount to child, and then calls payOut to direct the reward account to send that much Ether to child.child contains a fallback function (of course). This fallback function calls transfer on the DAO, and sends 99.9999% (or some other very large fraction) of its tokens back to parent. Remember that this also sends back the equivalent fraction of the child's paidOut, which has not yet had this new payout added to it. (The child can also send the newly-acquired Ether back to parent, here, too).payOut returns, and the child's paidOut gets that reward amount added to it.This leaves parent with 99.9999% of the original paidOut value of the tokens used (and the same quantity of tokens), and child with .0001% of the tokens, but 100.0001% of the paidOut value of the total tokens. It can now send these "tainted" tokens to another account to reset its paidOut and repeat the process.
The end result is a small quantity of tokens that carry with them a very large paidOut debt.
So let's bring this whole thing full circle.
With all of the above exploits, a perfect drain of the DAO is made possible. In the "funded" scenario, the only Ether safe from direct theft is that which is contained in the extraBalance of the DAO--which was approximately 345k Ether when funding closed. This Ether could only be brought back into the DAO's balance after the DAO had spend more than the total extraBalance on proposals.
Any balance in the rewardAccount, however: free for the taking.
Any balance in the DAOrewardAccount: free for the taking.
Any balance in the DAO itself... well, as we saw, that was pretty much free for the taking, too. In fact, it's likely that we didn't see the token tainting attack in that case because no one else participated in that split.
Let's assume, for this exercise, that others had participated in the split. As a hedge against this, the attack could taint some tokens using the above attack--and then when the voting period ends, send small fractions of that tainted balance to every voter on the split proposal. This would prevent everyone who was capable of splitting from executing the splitDAO call unless they dumped the tainted tokens and acquired new ones. In fact, given the nature of the tainting, it's even possible to "poison the well", so to speak--by sending tainted tokens to exchanges, where they'll be disseminated to many users. This dilutes the effect, but even a small adjustment to the paidOut is enough to break withdrawRewardFor when there's nothing in rewardAccount.
This gives rise to the "perfect split": a recursive splitDAO attack in which all no stalkers are capable of effectively executing splitDAO and obtaining tokens in the newly-created child DAO. This leaves the attacker with the sole ownership of tokens in the child DAO, giving him perfect control over it (and the ability to split into a DAO he controls, if he didn't do that in the first place).
There are a lot of red flags around the DAO v1.0 code. The authors clearly knew when to use .send vs when to use .call, but chose to use .call in a select few areas. The decision to make rewardAccount and DAOrewardAccount unrestricted... inexplicable. The fact that the token tainting attack and the splitDAO attack go together perfectly to ensure a flawless exit...
And then there's that failed funding failure mode.
Literally every mode had an escape hatch. Didn't make the funding target? Doesn't matter, you can still take all the funding for yourself.
Made the funding target? Cool, now you can take everything in the DAO, and prevent others from doing the same.
This is all further compounded by the constant claims from the Slock.it team that they had the "community" review the code--and yet no instances of the DAO's code can be found prior to it being referenced on the DAO's website, which only went up when the full marketing blitz was ready and waiting.
The initial investments into the DAO were likely from insiders involved in its creation. After all, this is a classic marketing technique: have your insiders pump up an offering (likes, comments, reviews, contributions, etc) to make it appear to have more initial momentum than it really has. This draws in more people, due to various social factors, not the least of which is the "fear of missing out." The DAO was deployed with a minimum token allocation of 50000000000000000000000, or 5,000,000 tokens (50,000 ether). Given the above opportunity to drain the DAO if it didn't reach its minimum, it would imply that the insiders didn't contribute more than 50,000 of their own ether, which is a fairly low bar.
So, we have a contract, written by people who clearly know how Solidity works, making a whole lot of mistakes in a lot of obvious places. We have associates of the authors claiming that the code was "reviewed by the community" prior to it being launched... but there's no evidence of this anywhere, and when I ask them for links to these posts, I'm ignored.
Suspicious.
Even so, it's all still very circumstantial. However, there does exist a public record of the development of the DAO's code: https://github.com/TheDAO/DAO-1.0
So, up next: a deep dive through the commit history of the affected parts. Let's try to put together the story of how this contract became so broken--intentionally or otherwise.
Stay tuned.
