sherlock-audit / 2023-01-optimism-judging

22 stars 8 forks source link

shw - No authorization on `finalizeWithdrawalTransaction` in `OptimismPortal` leads to chances of MEV attacks. #310

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

shw

medium

No authorization on finalizeWithdrawalTransaction in OptimismPortal leads to chances of MEV attacks.

Summary

No authorization on finalizeWithdrawalTransaction in OptimismPortal leads to chances of MEV attacks. A malicious actor can trigger the victims' cross-chain calls to either 1) conduct MEV attacks or 2) conducts unexpected reentrancy attack.

Vulnerability Detail

The finalizeWithdrawalTransaction of OptimismPortal.sol starts a cross-chain call if the correct proof is provided.

contract OptimismPortal is Initializable, ResourceMetering, Semver {

  function proveWithdrawalTransaction(
        Types.WithdrawalTransaction memory _tx,
        uint256 _l2OutputIndex,
        Types.OutputRootProof calldata _outputRootProof,
        bytes[] calldata _withdrawalProof
    ) external {
    // ...
    }
}

A malicious actor can submit the finalizeWithdrawalTransaction on behalf of others, even within a contract, call if provided with the correct proof.

Impact

  1. It's easier to launch sandwich attacks as the attacker can trigger the calls within flashloans.
  2. As Multichain-Dapps become popular, DeFi protocols integrate cross-chain messages in the service. Many cross-chain messages are privileged actions that would lead to unexpected outcomes (e.g., reentrancy attacks) if the attackers can trigger the calls at will.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/3f4b3c328153a8aa03611158b6984d624b17c1d9/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L160-L165

Tool used

Manual Review

Recommendation

As relaying transactions can be a huge attack vector, we've seen protocols (e.g. Yearn's stealth relayer) building mechanisms to mitigate the issue.

Recommend allowing users to set an optional relayer address. A restricted address can only trigger withdrawals if the relayer address is set.

This not only allows protocols to build easier mitigation of this issue, but it can educate new users about this potential attack vector.

rcstanciu commented 1 year ago

Comment from Optimism


Description: No authorization on finalizeWithdrawalTransaction in OptimismPortal leads to chances of MEV attacks.

Reason: This is a non-issue, but the suggestion to add an optional relayer address has some merit. It is intentional that the finalizeWithdrawalTransaction function is permissionless. This said, if a user would not like their withdrawal to be permissionlessly proven and executed, we could add support for this.

Action: Potentially add an optional relayer address to withdrawal transactions.