Parity Wallet Hack 2: Electric Boogaloo
Postmortem: The what, when, and how of the Nov 6th Parity Wallet Hack
TL;DR
The Parity Multisig WalletLibrary contract deployed 1 day after the first Parity Wallet Hack contained a bug that allowed anyone to execute initWallet. Someone experimenting with the previous exploit called initWallet and, subsequently, kill on the WalletLibrary, removing it from the blockchain. Because the actual wallet contract delegates all calls to this hard-coded WalletLibrary contract, it no longer has the logic to send funds.
Approximately 513k ETH (≈154 million USD) has been locked in the affected contracts. No funds were “stolen” per-say; only made unreachable. The user devops199 (whose name will go down in history 😄) did this accidentally while experimenting with the previous well-known exploit.
What’s Different from Parity Wallet Hack 1?
I’d recommend reading this technical breakdown on how the first hack went down.
The simplified version of Parity Wallet Hack 1 is as follows:
The Wallet contract is a simple contract that uses delegatecall to execute transactions using WalletLibrary ’s code within the context of the Wallet function. The WalletLibrary contract assumes that it will be called in the context of a contract that does have state that it can modify (namely m_numOwners) and this is the core assumption that caused the recent disaster. In the first Parity Wallet Hack, the hacker changed the state of various Wallet contracts by delegating a call to initWallet, setting themselves as the owner of the Wallet contract and then withdrawing funds normally.
How and Why
In this case, the internal state of the Wallet contracts that have been deployed hasn’t been changed; what changed was the internal state of the WalletLibrary contract. The WalletLibrary contract contains a state variable m_numOwners (along with m_owners and similar) that it expects to be shadowed by the calling contract’s own state. The rest of its state is globally shared between all Parity Multisig Wallets that hardcode its address.
After deploy, the WalletLibrary contract is simply uninitialized. m_numOwners is 0. This means that the only_uninitialized modifier (which would act correctly when called in the context of an initialized Wallet contract) always passes. If the WalletLibrary isn’t executed in a Wallet contract’s context, m_numOwners is 0, allowing anyone to call methods that this modifier guards, one of which is initWallet.
Anyone could have called this function at any time in the past 110 days since it was deployed, but it obviously went undetected until now.
Result
Here’s an example of an affected wallet (the Polkadot ICO wallet, containing ~90m USD in ether). Note line 451 where the WalletLibrary address is hard coded. Assuming all calls to this address fail (comments removed and knowing that the constructor isn’t actually included in the contract bytecode), this is what the contract looks like right now:
contract Wallet is WalletEvents {
function() payable {
if (msg.value > 0)
Deposit(msg.sender, msg.value);
} function getOwner(uint ownerIndex) constant returns (address) {
return address(m_owners[ownerIndex + 1]);
} address constant _walletLibrary = 0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4; uint public m_required;
uint public m_numOwners;
uint public m_dailyLimit;
uint public m_spentToday;
uint public m_lastDay;
uint[256] m_owners;
… which doesn’t do much at all, and certainly doesn’t send funds anywhere.
Timeline
T= Jul 20, 2017 04:39:46 PM UTC
The WalletLibrarycontract was deployed following the aftermath of Parity Hack 1. You can see the verified source code on Etherscan. Of note are lines 215 (only_uninitialized) and 225 (kill).
T= Nov 06, 2017 02:33:47 PM UTC
Transaction 0x05f71e1b was sent to WalletLibrary, calling the initWallet method. This transaction succeeded in making 0xae7168deb525862f4fee37d987a971b385b96952 the sole owner.
T= Nov 06, 2017 03:25:21 PM UTC (+51 minutes from previous tx)
Transaction 0x47f7cff7 was sent to WalletLibrary, calling the kill method with 0xae7168deb525862f4fee37d987a971b385b96952 as the beneficiary address.
T= Nov 06, 2017 03:54:34 PM UTC (+29 minutes from previous)
devops199 created parity#6995 documenting the transactions.
T= Nov 06, 2017 04:33:00 PM UTC (+39 minutes from previous)
devops199 posts a link to the issue in the parity gitter channel.
and is understandably worried about the consequences.
T= Nov 6th, 2017 07:51:00 PM UTC (+ 3 hours and 18 minutes)
Parity releases a warning and states that they’re investigating.
Possible Resolution Avenues
Off the top of my head, here are the three possible approaches for recovering funds:
- Hardfork, ala DAO Hack, which may or may not result in another “classic” fork. If this is done during the planned Constantinople hardfork, it may proceed particularly smoothly.
- Break cryptography and deploy a new contract to the hardcoded WalletLibrary address. Which is, at the moment, cryptographically infeasible before the heat death of the universe.
- Implement EIP156, which provides a community-controlled way to recover funds during events such as these. While this EIP doesn’t specifically solve the issue (as noted by sciyoshi), there is at least some precedent for the community supporting a method like this to avoid a hard fork.
In general, though, it seems the funds are locked for the foreseeable future.
Possible Prevention Measures
How could this have been prevented? The primary cause of the bug seems to be twofold:
- The quickly-patched WalletLibrary code was not audited after the first Parity Wallet Hack.
- The library pattern hid the fact that WalletLibrary is an actual contract with internal state. This encouraged an assumption that it would only be called in the context of a Wallet contract. The Solidity library construct would have made this bug much more obvious; Solidity library contracts aren’t allowed to have internal state, so the fact that anyone could call
initWalletwould have been much more obvious. Additionally, they wouldn’t be able to implement ownership (internal state), which is what made this code feel safe; theonlyOwnersmodifier was expected to perform correctly.
Some possible prevention measures (could have been):
- After deploying the WalletLibrary contract, anyone could have called
addOwnerwith a null owner, effectively locking the contract from change (when called directly). It would still function correctly as a library when the Wallet contract state shadows the null-owner state of the WalletLibrary contract. - Placing the
killlogic within a contract that was designed to be immutable is a suspect decision. Again, it seemed safe because of theonlyOwnersmodifier and Wallet context assumption, which is whykillwasn’t placed directly on the Wallet contract itself. At least in the future, monitoringselfdestructlines will be part of every code audit. - In general, the globally shared state of the WalletLibrary contract was a curious decision. It didn’t directly contribute to this hack, but transparently sharing state between MultiSig contracts like this just feels weird.
Who’s Affected?
Here’s a list of the wallets affected by the deletion of WalletLibrary, ordered by net worth.
Of note:
- the Polkadot ICO wallet 0x3bf with 306k ETH
- an ICONOMI-affiliated wallet 0x376 with 115k ETH
- a MUSICONOMI-affiliated wallet 0xc7c with 16k ETH
if you know the owners of the other wallets and don’t mind listing them, let me know!
Conclusion
Nice. The situation is still developing, to an extent. Parity will most likely announce an update on Twitter.
To be clear, I place exactly 0 blame on devops199, and I don’t think anyone else should either.
On that note, if this post made you happy, informed, or some other vaguely positive emotion, please smash that 👏 button because I feed on imaginary internet points.
Thanks to Dror Liebenthal for proof reading the post.
