- Type: Report
- Network: Ethereum
- Total lost: 400K USD (bounty price)
- Category: Reinitialization
- Vulnerable contracts:
-
- Vulnerable implementation: 0x3e2198a77fc6b266082b92859092170763548730
- Attack transactions:
-
- None
- Attacker Addresses:
-
- None
- Attack Block:: -
- Date: Sept 19, 2022 (public disclosure)
- Reproduce:
forge test --match-contract Report_ArbitrumInbox -vvv
- Craft an evil
_bridge
contract - Call
initialize
setting the_bridge
to be your malicious contract.
The Inbox is part of the Arbitrum Bridge between ETH and Arbitrum. The Inbox takes some messages and forwards them to the Bridge contract.
To do this, it takes a reference to the bridge address in its initialize
. As hinted by this method, the whole contract is behind an Universal Upgradable Proxy.
The problem can be found in the initialization of the implementation contract. While the initialize
method is correctly protected by a initializer
guard, which makes sure that this method can only be called once, it does so by using flags which are in position 0x00
and 0x01
in the storage. But the postUpgradeInit
method, called after initialization, deletes the first three slots!
This results in the contract being marked as not-initialized and deleting its reference to the _sequencerInbox
. This last variable is not used, so it's not actually a problem. But now that the contract is marked as not-initialized, anyone can call initialize
again with their own _bridge
address!
function initialize(IBridge _bridge, ISequencerInbox _sequencerInbox)
external
initializer
onlyDelegated
{
bridge = _bridge;
sequencerInbox = _sequencerInbox;
allowListEnabled = false;
__Pausable_init();
}
/// @dev function to be called one time during the inbox upgrade process
/// this is used to fix the storage slots
function postUpgradeInit(IBridge _bridge) external onlyDelegated onlyProxyOwner {
uint8 slotsToWipe = 3;
for (uint8 i = 0; i < slotsToWipe; i++) {
assembly {
sstore(i, 0)
}
}
allowListEnabled = false;
bridge = _bridge;
}
An attacker can quite easily exploit this by taking advantage of a call the Inbox makes to the Bridge which sends value, specifically to the method enqueueDelayedMessage()
(follow depositEth
in the vulnerable contract for the full path). An attacker could have forwarded all ETH deposits from the inbox to their own evil contract.
Maybe more interesting than the exploit itself is how the vulnerability came to be. Two different commits where needed to break the contract:
- c33765fa66d74733ab740c0f0cbdf27a05d1d985 on Feb 18, 2022 introduced the wiping of the slots. This nevertheless was not vulnerable: even though the slots where wiped, they were replaced by another flag in the
initialize
method:if(address(bridge) != address(0)) revert AlreadyInit();
. This explains why it was safe to delete these slots, as they are not needed anymore. - 2631e1e0a4767ef95898ccdca727d61fa1353031 on Aug 1, 2022. 6 months after the commit that removed the slots, it was likely forgotten that the
address(bridge)
check replaced theinitialize
flags, and the check was removed; making the contract vulnerable.
- Be careful when wiping up slots.
- Be careful when removing "useless" checks.
- Test deploy conditions, like
should not be able to reinitialize contract