Today we witnessed a possible major attack on the Parity MultiSig contract. This follows a previous hack in July. Estimated losses may total over 500,000 ETH ($150 million USD), including over 300,000 ETH from the Web3 Foundation team.
While others have followed the story—including this excellent post by Springrole—I wanted to focus on the technical details of what happened.
A quick recap
The Parity MultiSig wallet is structured as a very thin contract that delegates all calls to a main library contract through a delegatecall in its fallback function:
// gets called when no other function matches
function() payable {
// just being sent some cash?
if (msg.value > 0)
Deposit(msg.sender, msg.value);
else if (msg.data.length > 0)
_walletLibrary.delegatecall(msg.data);
}
It’s critical to understand how delegatecall works.
This method runs the code from the library in the context of the current contract’s store. Whenever each thin wallet contract delegates to the library, the executed library code will always refer to the store of the calling contract, not the library’s. It will also preserve the original message sender and value, effectively acting as if the library’s code were inlined in the calling contract.
This allows for each deployed wallet to re-use the same code, greatly reducing gas costs for deployment. This is a great feature, and we’re developing similar ideas for the zeppelin_os kernel.
Note that, since all functions in the library need to be called from the thin wallet contracts, they all need to be public. This includes the initWallet method, which was used in the constructor to initialize the wallet’s state.
July’s attack exploited this fact, given that it was public and had no modifiers to prevent anyone from invoking it. The attacker simply invoked initWallet on its targets, resetting the ownership to an address he controlled, and then siphoned the funds to his account.
The fix
In order to protect the initWallet method, a modifier was added to ensure that it could only be invoked once, during construction:
modifier only_uninitialized { if (m_numOwners > 0) throw; _; }
This way, if an attacker attempted to invoke initWallet via the delegatecall mechanism in any of the thin wallet contracts, it would be rejected by this modifier.
The new vulnerability
Note that the new modifier checks the contract’s store to determine whether it was already initialized. This effectively blocks any attempts to call initWallet from a contract that has already set m_numOwners.
In this new scenario, instead of invoking initWallet on any wallet contract, the attacker called initWallet in the library contract itself. Let’s trace what this implies:
- The library contract, as any regular contract, sees a call to
initWalletand checks its modifiers. - Since
m_numOwnersis zero, because the library itself was never initialized, the check passes. - The
m_ownersof the library are reset to whatever the attacker chose in the new initialization.
In other words, the vulnerability occurred because the shared library was not deemed as another contract on the blockchain with its own state. Even though library contracts are used to deploy shared code in a single instance to the blockchain, they are still full-fledged contracts with data stores of their own, making them as susceptible to attack as any other contract.
A new attack
Instead of targeting individual wallet contracts, it appears that the latest attack hit the shared library, instantly affecting all wallet contracts that depended on it.
The attack itself was simple: kill the library contract. This caused all of the thin wallet contracts that relied on it to start delegating their calls to an address without any code, thus becoming unable to execute any action. In other words, all wallet contracts became instantly frozen.
It’s worth mentioning that allowing any ether-receiving contract to be destroyed is a recipe for disaster, since any ether accidentally sent to it after its destruction becomes irretrievable. However, the problem here was more fundamental: a contract designed as a library should never have been able to have its own state altered. Solidity even enforces this by forbidding libraries to have non-constant state fields.
A possible solution for this vulnerability could be to check on every call that the address in context (aka this in Solidity) is not the library’s own address, hence only allowing calling the library contract through delegatecalls.
The same conclusion
Unfortunately, the latest attack yielded the same result as the one five months ago.
It is important to note that the technique of abstracting logic into a shared library can be quite useful, though. It helps improve code reusability and reduces gas deployment costs. This attack, however, makes clear that a set of best practices and standards is needed in the Ethereum ecosystem to ensure that coding patterns are implemented effectively and securely. Otherwise, the most innocent-looking bug can have disastrous consequences.
Once again, a custom implementation of what should have been a standard pattern has backfired into a multimillion-dollar loss.
We have been working hard at Zeppelin to design zeppelin_os, a platform for the secure development of smart contracts. It allows anyone to leverage the power of the EVM without risking security.
Our goal is to provide built-in secure implementations of common development patterns, such as the one that caused the latest vulnerability. In this case, a verified implementation of the library pattern would have prevented any mutable state in the library to be targeted. If the library was compromised, the upgrade management mechanism could have switched to a safe implementation, rather than leaving hundreds of depending contracts unusable.
You can follow the development of zeppelin_os on our blog, go through the latest version of the whitepaper, or join the ongoing discussion on our Slack channel.
