Arbitrum's Seatbelt fastening after Tornado Cash attack

The Tornado Cash Governance takeover was all the rage last week. All crypto-space witnessed how an attacker managed to pass a malicious proposal through one of the most hardcore crypto communities and steal ~2.7M USD in governance tokens.

Coinspect was the first to provide a detailed, step-by-step reproduction of how the attacker achieved this on Thursday last week. All the technical details of the attack can now be found on our learn-evm-attacks repository.

After publishing the reproduction, we realized that most likely other governance protocols were vulnerable to this kind of bait-and-switch proposal. We dived into some popular governance protocols and were surprised to see that Arbitrum
seemed vulnerable to the exact same attack.

Do not worry: Arbitrum’s governance is not at immediate risk. At Coinspect we think it is crucial not to spread uncertainty or doubt: while the issue exists, Arbitrum has mitigations and safeguards in place. Security works in layers and we had found a vulnerability in one of the layers. With that said, let’s analyze our finding, how we reported it to Arbitrum, their response and possible mitigations for the issue.

To understand the attack you must know two important facts:

1. An attacker can present a proposal that looks beneficial to the protocol, SELFDESTRUCT it and replace it by a malicious proposal after it has gathered enough votes.
2. Some governance protocols use DELEGATECALL to call proposals, which give the proposals full control over the executor’s state (eg: an owner variable can be replaced by the proposal).

If you want to dive deep into how these facts are leveraged to conduct the attack, read our explainer of the Tornado Cash attack.

All in all, for a governance protocol to be vulnerable:

1. An executor contract must use delegatecall to execute proposals.
2. The executor must have security-critical state.
3. That security-critical state must not be protected.

Arbitrum’s executor behaves exactly like this. See its execute method:

   function execute(address upgrade, bytes memory upgradeCallData)
        public
        payable
        onlyRole(EXECUTOR_ROLE)
        nonReentrant
    {
        // OZ Address library check if the address is a contract and bubble up inner revert reason
        address(upgrade).functionDelegateCall(
            upgradeCallData, "UpgradeExecutor: inner delegate call failed without reason"
        );

        emit UpgradeExecuted(upgrade, msg.value, upgradeCallData);
    }

In a way, Arbitrum’s case is even worse than Tornado Cash: the storage at risk is the initialized flag of an upgradable contract that has the power to upgrade all other Arbitrum contracts. This means that an attacker that manages to successfully pass a malicious proposal can take control of all Arbitrum contracts.

We made a proof of concept using fork-testing to make sure we were right. The proof of concept worked and we were on total control of Arbitrum’s Upgrade Executor contract on our fork. At this point, we had to let Arbitrum know.

As a security firm, we always try to contact the project immediately upon a finding. The only official point of contact for vulnerabilities we could find was
Arbitrum’s Bug Bounty Program. A detailed report was sent along with a proof of concept.

Seatbelt mitigation: detecting mutating contracts

After two days we got a response back from Arbitrum. They were aware of the issue and had mitigations ready to be sent to production. Our report on Immunefi was closed due to the issue being already known.

Interestingly, their mitigation strategy is completely off-chain and relies on a toolkit called Seatbelt. This tool was created by Uniswap and is a proposal analyzer of sorts: it performs different checks on proposals and produces a report which is intended to help voters understand what a proposal does.

Just one day after the Tornado Attack, Seatbelt had PRs both on its original Uniswap repository and Arbitrum’s fork checking for the existence of SELFDESTRUCTs and DELEGATECALLs in proposals. Note that Arbitrum’s fork has not integrated the change yet.

Protecting critical variables

While doing our research, we found that governance protocols protect their storage in several different ways. One of the strongest ones we found was also one of the simplest: just making sure that the critical storage slots do not change. This is what AragonOS does:

    modifier protectState {
        address preKernel = address(kernel());
        bytes32 preAppId = appId();
        _; // exec
        require(address(kernel()) == preKernel, ERROR_PROTECTED_STATE_MODIFIED);
        require(appId() == preAppId, ERROR_PROTECTED_STATE_MODIFIED);
    }

Unfortunately, this simply does not work to protect more complex data structures such as mappings.

Of course, the simplest solution is not having any storage on the executor, so DELEGATECALL can be used just to preserve the sender’s context. But this makes having upgradable executors impossible.

Best of both worlds

You might have noticed: Aragon’s solution protects the state, but does not protect from mutating proposals. Meanwhile, Seatbelt reduces the risk of having mutating proposals but does not protect the storage itself.

We believe a two-pronged approach would be the most robust, and would encourage Seatbelt users to also include a clause like Aragon’s in their executors. SELFDESTRUCT and DELEGATECALL are not the only ways a proposal might sneak in malicious storage changes, so additional protections should be put in place.

Another possible solution against mutating proposals would be storing the hash of the code of the proposal when it is submitted. On every vote and at execution time, the code of the proposal must remain unchanged. If the hash changed, the protocol has fallen victim to a mutating proposal and should abort the execution and drop the proposal. This, plus directly protecting important storage slots like Aragon does, seems like the strongest mitigation as it is enforced by the chain, unlike Seatbelt.

Other governance protocols at risk?

We believe many other might be vulnerable to this specific attack vector. We encourage existing protocols to consider the mitigations discussed here and implement them where necessary. At Coinspect, we are dedicated to the security of decentralized systems and can assist in auditing your protocols and enforcing safeguards. So reach out to us today.