Advice for DAO 2.0s

With the Ethereum hard fork successful, it's a good time to take stock.

I'm expecting 40% of TheDAO funders to take their money back, crying quietly and vow never to do something like that again.

Of the remaining 60%, some large percentage seem likely to say "That was fun! Let's do another one!"

I'd love for that 60% to have a safer experience next time around, and there are a number of simple things that DAO contract authors can do to make things better for their customers/shareholders/token holders.

In particular, I want to discuss simplicity of architecture, security considerations and legal / governance questions.

Security Considerations: Calls, State, Tokens, Underflow

There are four basic security vulnerabilities that DAO smart contracts seem to commonly encounter -- recursive calls, state mismanagement, token mismanagement, and over/underflow problems.

I'm going to write some innocuous DAO code that has all four vulnerabilities, and then we can dissect it.

// Do Not Copy This Code. It is riddled with problems.
bool withdrawingMutex;  
mapping (address => uint) tokens;  
uint globalTokens;

function withdraw(uint amount) returns(bool) {  
   if(withdrawingMutex) { return false; }
   withdrawingMutex = true;
   if (amount > tokens[msg.sender]) { return false; }
   if (!msg.sender.call.value(amount)()) { return false; }
   tokens[msg.sender] -= amount; 
   globalTokens -= amount;
   withdrawingMutex = false;
}

function sendTokens(address to, uint amount) returns(bool) {  
   if (tokens[msg.sender] >= amount) { 
      tokens[to] += amount;
      tokens[msg.sender] -= amount;
      return true;
   }
   return false;
}

First, some things this code has going for it: it mutexes withdraws, so you can't recursively call withdraw. It tries to be careful about token underflow (although it thinks of underflow as using more tokens than are available, not as an engineering-level risk). Despite that, these functions allow a full withdrawal from the contract for any token holder.

Here is the attack:

  1. Assume we have balance 10 tokens, each redeemable for 1 ether
  2. Assume the contract has 100,000 ether.
  3. Attacker calls contract.withdraw(10)
  4. Attack wallet default function is called
  5. It calls contract.sendTokens(otherAddr, 10)
  6. contract.sendTokens reduces tokens[msg.sender] to 0
  7. contract.withdraw(10) reduces tokens[msg.sender] to -10 or 2^256-10
  8. contract.withdraw(999990) can now be called.

A variant of this call stack was used to magnify TheDAO attack.

This attack can be extended to mess with the globalToken supply as well.

Simple Swaps and Unsigned Integers

The simplest way to fix the above code is to swap a few lines in the withdraw function -

tokens[msg.sender] -= amount;  
globalTokens -= amount;  
if (!msg.sender.call.value(amount)()) { return false; }  

This will get rid of the token underflow error, but not the chance for other underflows elsewhere. Changing the type to int could be used for the mapping. For some reason that I do not understand, this is not common in solidity documentation.

An int carries the nice property that value comparison checks work if there is an underflow. Even if the two lines above were not swapped, an int would limit the attack vector to the value held by the original depositor.

The above code also could allow the total token balance to get out of sync -- if the recipient wallet call fails, perhaps due to a legitimate lack of gas in the function stack, then the owner loses tokens.

To fix that, the amounts need to be added back in after. This level of care is not uncommon in quality contracts, but I have not seen a contract that checks for overflows during the re-add. If the contract does not call out elsewhere in Ethereum, then there is no need to worry about an overflow. But if it does, then an overflow is possible.

What is the State of The Contract?

The withdraw function returns false rather than throws. This means that the withdrawingMutex can be set to true, a test can fell and then the contract is wedged; no future withdrawals are possible.

throw would probably be better here, but if the contract exists in a multi-contract constellation, even throw may not be enough to unwind all state changes. There is no clear simple way to fix all these state problems, it requires engineers to trace call paths and make sure nothing has been missed.

Token State Concerns

ERC20 function examples keep track of tokenSupply in a global uint and then return the uint when tokenSupply is called. This makes sense from a gas and complexity framework - iterating through all tokenHolders can be difficult. In fact, with the solidity mapping implementation, it can be impossible to iterate through and check if the token balances are only kept in a map -- maps cannot be iterated by key in solidity.

This is terribly, terribly optimistic. Every DAO should have a means (even if the gas costs are high) to validate the token supply manually and have a mitigation plan put in place.

And, those token values should be ints: underflows are nasty.

Overflow

The contract does not check for overflow in sendTokens: while it's typical to declare token mappings as uint and therefore uint256, there have been some token attack vectors that could get geometric growth out of token exploits.

Alternately, a token system may use something smaller like uint32, making an overflow attack more feasible.

Legal Considerations

TheDAO story is not nearly over -- the lawsuits have not yet begun! Expect years more fun before everything is settled. I want to hammer on one particular area -- custody -- that has been worrying me, and take a few angles on it.

Disclaimer I'm not a lawyer, and this is not legal advice.

Custody Questions

Smart Contracts can be created so that they have no custodian -- or, put another way, so that the only custodian is the code itself. One very nice property they receive is that no parties can be induced, through legal or other means, to take an action about the contract.

Inducement isn't the only concern -- legal responsibility for actions taken or not taken should be a concern as well.

In my refund contract recommendations post, I recommended strongly that the refund contract have no custodians, instead that the refund protocol be encoded in the contract itself.

Unfortunately for the escrow agents overseeing TheDAO, they have the ability to block or approve withdrawals. This is a very high risk situation to be in. I would guess that they are not licensed escrow agents in each jurisdiction that DAO token holders live in, for instance.

I could go on, but I don't want to - there are too many bad outcomes for these custodians.

Experienced Bitcoin attorneys like Patrick Murck and others have also noted that Ethereum developers run the risk of being seen as having custody when they muck about with moving funds between accounts.

I would urge DAO creators to leave themselves out of the custodian question altogether by making sure that the specified actions are encoded into software.

Upgradability and Custody

However, this raises a question -- what happens when something goes wrong? Software will need upgrading. Bugs need fixing. I often recommend a 'pause' and 'upgrade' functionality built into contracts. Best practices like this are supported by dappsys and other quality contract offerings.

If a party or set of parties can upgrade a contract, then they may hold liability for custody. On the other hand, if nobody can upgrade a contract, it will likely be not useful at some point in the future.

There aren't easy answers here. I would urge all DAO creators to dialogue extensively with their communities about what is preferred, and about truly bad scenarios, including multiparty, multijurisdiction lawsuits, criminal collusion, and deniable attacks. We unfortunately don't know all the ins and outs of doing a good job on this yet, but there are some rookie mistakes that can be avoided just by discussion.

Ego and Simplicity

One of the most objectionable architectural decisions in TheDAO was adding the concept of child DAOs. These were created for two reasons, as far as I understand:

  1. People thought that it would be nice to be able to pull ether out but still have rights to investment returns already made by TheDAO before they withdrew.
  2. People thought it would be nice to have a ready-made structure for participants to 'split' and be able to keep using.

If I had been king of TheDAO specification, I would not have included child DAOs. They vastly complicated the attack surface and ability to reason about outcomes. At least 60% of the identified game theoretic attacks have to do with children, the major exploit came from the ability to split, and there are still problems with tokens that aren't well understood.

I think by and large the issues come from a desire to be 'cool' or look smart. We could look less smart and be safer by choosing one of the following instead: withdrawals from TheDAO lower ownership percentage pro-rata, or any withdrawal liquidates the account and gives up future earnings. In the first case, there are some questions, tough questions, about how to value held investments (proposals).

The second case would be my preferred choice. If you're out and don't want to sell your tokens, liquidate -- this provides an arbitrage-maintained floor for token value, and would encourage participants to stay in. If you want out and the token market is good, great. The token markets would get increased liquidity, and the value would remain pegged to the eth left in the contract plus some market notion of returns on paid proposals.

If users wished to form a new DAO, nothing would stop them from withdrawing and publishing their own contract, no nested DAOs required.

I urge DAO creators to try and keep things as absolutely simple as possible. Strive for it.

Consulting

As always, I and my team are available for smart contract creation, auditing and specification: contact me at [email protected].

Peter Vessenes

Read more posts by this author.

Subscribe to Peter Vessenes

Get the latest posts delivered right to your inbox.

or subscribe via RSS with Feedly!