TrustSec has contributed to the Optimism ecosystem both in contributing to contests, and audits and two paid bug bounties. In this post, they talk about the security of Optimism and some of the more interesting bugs they discovered. There is a lot of background one the one bug they go through in the blog post.
L2s need a way to communicate with the L1s and vice versa. In Optimism, going from the L2 to the L1 requires a Merkle proof from the trusted L2 state root. When going from Ethereum to Optimism, OptimismPortal is used which emits an event that is translated to an ETH minting call. Both of these give the capability to send arbitrary data between the chains.
There is a limitation to what's above though: calls can become stuck. So, the CrossDomainMessenger (XDM) is where the other bridging functionality, like ERC20/ETH bridge and the ERC721Bridge are implemented. XDM supports resending failed messages via a mapping of either successful or failed messages.
On the revert flow, there is a very important check... If the XDM failed then the ETH is has already been supplied. Additionally, the successfulMessages mapping prevents the same withdrawal from being executed more than once.
When making a call from the L2, the default L2 sender is 0xDEAD. The variable is xDomainMsgSender. On a cross-chain message, this is set to the calling user. In effect, this acts as a reentrancy protection as well. At the end of the call, the storage value is set back to the default.
The audit they did was for the SuperchainConfig contract upgrade. This was a single program that allowed for designated roles to pause or unpause a core contract. During this upgrade, they manually switch off the initialized bit of the contract to allow for recalling initialize() instead of using reinitializer modifier. Such a small line of code seems so simple. So, what's wrong?
The xDomainMsgSender is 0xDEAD at all times except during the withdrawal process. Within the initialization code (which gets retriggered), this value is set to 0xDEAD but actually defaults to zero. Normally, this would be fine (since it should be a NOP) but that's NOT true in the context of the withdrawal code!
Upgrades are permissionless once enough signers have agreed to the upgrade. Here's the flow of the attack:
-
Store a failed delivery to the smart contracts
failedMessages mapping.
-
Wait for the upgrade to be in the mempool and ready to go.
-
Reattempt the failed delivery:
-
Perform the upgrade with the parameters. Now, the msgSender is set to
0xDEAD.
-
Reenter into
relayMessage passing in the withdrawal request. This will succeed because the DEFAULT 0xDEAD address was set back.
-
The msgSender is set to L2Sender again when it shouldn't be.
-
A double withdrawal has occurred because of the double setting of the xDomainMsgSender global variable. The stolen amount comes from any failed withdrawals.
The setting of the xDomainMsgSender global variable doesn't seem important until you have the better context of what it does and why it's important. It's crazy how this reentrancy/replay protection was touched by this simple upgrade code. What an awesome find!