Anyone Could Initialise the Wallet — JavaScript Bug Hunt

Briefing

Modelled on the Parity multisig wallet hack of July 2017 — the first of Parity's two 2017 incidents. The wallet contracts forwarded unknown calls to a shared library, and the library's initWallet function, meant to run once at deployment, could be called again by anyone. Attackers re-initialised three wallets with themselves as the sole owner and withdrew roughly 150,000 ETH.

This project is a reconstruction in plain JavaScript: dispatch.js forwards any method name to the library (as the contract's fallback did), and wallet.js lets initWallet overwrite the owners of a wallet that is already set up.

Fix initWallet so a wallet can be initialised exactly once.

Bug report

BUG-PARITY-0719 · Priority: Critical (funds at risk) · Reported by: white-hat group

Wallet state: { owners: [], required: 0, balance, initialised }.

• initWallet(wallet, sender, owners, required) sets owners/required and marks the wallet initialised — ONLY if it is not initialised yet. A second call, from anyone (including an existing owner), throws Error("already initialised") and changes nothing.
• deploy(owners, required, balance) creates a wallet and initialises it.
• execute(wallet, sender, amount) withdraws: sender must be an owner (else throws Error("not an owner")), amount <= balance (else throws); returns the new balance.

Observed: dispatch(wallet, attacker, "initWallet", [[attacker], 1]) makes the attacker the only owner, and the next execute drains the wallet.

Logs

[chain] tx 0xeef1… initWallet([0xb3764…], 1) from 0xb3764… -> ok
[chain] tx 0x0e0f… execute(0xb3764…, 26793 ETH) from 0xb3764… -> ok

The code as shipped

src/wallet/wallet.js (editable)

exports.initWallet = function (wallet, sender, owners, required) {
  wallet.owners = owners.slice();
  wallet.required = required;
  wallet.initialised = true;
};

exports.deploy = function (owners, required, balance) {
  var wallet = { owners: [], required: 0, balance: balance, initialised: false };
  exports.initWallet(wallet, owners[0], owners, required);
  return wallet;
};

exports.execute = function (wallet, sender, amount) {
  if (wallet.owners.indexOf(sender) === -1) throw new Error("not an owner");
  if (amount > wallet.balance) throw new Error("insufficient balance");
  wallet.balance -= amount;
  return wallet.balance;
};

Read-only context: src/wallet/dispatch.js.