Withdraw, Then Withdraw Again — JavaScript Bug Hunt

Briefing

Modelled on The DAO attack (June 2016): The DAO's contract sent ether to a caller before updating the caller's balance. The recipient was itself a contract whose receive hook called straight back into the withdrawal, and every nested call still saw the untouched balance. Roughly a third of The DAO's funds were drained, and Ethereum hard-forked to reverse it.

vault.js has the same order of operations. The re-entrant recipient is modelled deterministically in wallets.js: its receive calls withdraw again a fixed number of times.

Fix withdraw so a nested call can never pay out the same balance twice.

Bug report

BUG-REENTRY · Priority: Critical (funds drained) · Reported by: audit

vault.withdraw(who, wallet):

• pays the caller's whole balance by calling wallet.receive(amount, vault) once and returns amount
• a balance of 0 returns 0 without calling receive
• receive may call vault.withdraw again (re-entrancy); that nested call must see the balance as already paid out and return 0
• after any sequence of calls, vault.reserve equals the sum of the balances still held
• if receive throws, the withdrawal is undone — the balance and the reserve are restored — and the error propagates

Observed: an attacker with a balance of 10 re-entered three times, was paid 40, and the honest depositor's 30 could no longer be withdrawn.

Logs

[vault] withdraw mallory 10 (depth 4)
[vault] reserve 40 -> 0, balances { mallory: 0, alice: 30 }
[vault] withdraw alice: Error: insufficient reserve

The code as shipped

src/vault/vault.js (editable)

exports.createVault = function () {
  var vault = { balances: {}, reserve: 0 };

  vault.deposit = function (who, amount) {
    vault.balances[who] = (vault.balances[who] || 0) + amount;
    vault.reserve += amount;
  };

  vault.withdraw = function (who, wallet) {
    var amount = vault.balances[who] || 0;
    if (amount === 0) return 0;
    if (amount > vault.reserve) throw new Error("insufficient reserve");
    wallet.receive(amount, vault);
    vault.reserve -= amount;
    vault.balances[who] = 0;
    return amount;
  };

  return vault;
};

Read-only context: src/vault/wallets.js.