paint-brush
Parity Wallet Hack 2: Electric Boogalooby@mattcondon
4,447 reads
4,447 reads

Parity Wallet Hack 2: Electric Boogaloo

by Matt CondonNovember 8th, 2017
Read on Terminal Reader
Read this story w/o Javascript
tldt arrow

Too Long; Didn't Read

The <a href="https://hackernoon.com/tagged/parity-multisig" target="_blank">Parity Multisig</a> WalletLibrary contract deployed 1 day after the first Parity Wallet Hack contained a bug that allowed anyone to execute <code class="markup--code markup--p-code">initWallet</code>. Someone experimenting with the previous exploit called <code class="markup--code markup--p-code">initWallet</code> and, subsequently, <code class="markup--code markup--p-code">kill</code> on the WalletLibrary, removing it from the <a href="https://hackernoon.com/tagged/blockchain" target="_blank">blockchain</a>. Because the actual wallet contract delegates all calls to this hard-coded WalletLibrary contract, it no longer has the logic to send funds.

Companies Mentioned

Mention Thumbnail
Mention Thumbnail

Coin Mentioned

Mention Thumbnail
featured image - Parity Wallet Hack 2: Electric Boogaloo
Matt Condon HackerNoon profile picture

Postmortem: The what, when, and how of the Nov 6th Parity Wallet Hack

behold my photoshop skills

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 Parity Wallet Hack Explained_A vulnerability was found on the Parity Multisig Wallet, that allowed a hacker to steal over 150,000 ETH._blog.zeppelin.solutions

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 [WalletLibrary](https://etherscan.io/address/0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4)contract 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](https://etherscan.io/address/0xae7168deb525862f4fee37d987a971b385b96952) the sole owner.

https://etherscan.io/tx/0x05f71e1b2cb4f03e547739db15d080fd30c989eda04d37ce6264c5686e0722c9

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](https://etherscan.io/address/0xae7168deb525862f4fee37d987a971b385b96952) as the beneficiary address.

https://etherscan.io/tx/0x47f7cff7a5e671884629c93b368cb18f58a993f4b19c2a53a8662e3f1482f690

T= Nov 06, 2017 03:54:34 PM UTC (+29 minutes from previous)

devops199 created parity#6995 documenting the transactions.

https://github.com/paritytech/parity/issues/6995

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.

I can’t seem to link to gitter messages, so a screenshot will have to do.

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:

  1. 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.
  2. 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.
  3. 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:

  1. The quickly-patched WalletLibrary code was not audited after the first Parity Wallet Hack.
  2. 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 initWallet would have been much more obvious. Additionally, they wouldn’t be able to implement ownership (internal state), which is what made this code feel safe; the onlyOwners modifier was expected to perform correctly.

Some possible prevention measures (could have been):

  1. After deploying the WalletLibrary contract, anyone could have called addOwner with 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.
  2. Placing the kill logic within a contract that was designed to be immutable is a suspect decision. Again, it seemed safe because of the onlyOwners modifier and Wallet context assumption, which is why kill wasn’t placed directly on the Wallet contract itself. At least in the future, monitoring selfdestruct lines will be part of every code audit.
  3. 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.

but wait there’s more!

Of note:

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.

ps. I was the first person to think of advertising on the contract’s old address.

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.