Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

ck - Block number is not an input to proveWithdrawalTransaction in OptimismPortal #233

Open
github-actions bot opened this issue Feb 20, 2023 · 0 comments
Labels
Reward A payout will be made for this issue Specification An issue related to the specification (low severity)

Comments

@github-actions
Copy link

ck

low

Block number is not an input to proveWithdrawalTransaction in OptimismPortal

Summary

According to the Withdrawals spec, a block number is one of the submitted inputs to the OptimismPortal which is not the case.

Vulnerability Detail

The documentation says:

"A relayer submits the required inputs to the OptimismPortal contract. The relayer need not be the same entity which initiated the withdrawal on L2. These inputs include the withdrawal transaction data, inclusion proofs, and a block number. The block number must be one for which an L2 output root exists, which commits to the withdrawal as registered on L2."

In the contract, the _l2OutputIndex is the one used to check for existence of the outputRoot and not the block number.

   function proveWithdrawalTransaction(
        Types.WithdrawalTransaction memory _tx,
        uint256 _l2OutputIndex,
        Types.OutputRootProof calldata _outputRootProof,
        bytes[] calldata _withdrawalProof
    ) external {
        // Prevent users from creating a deposit transaction where this address is the message
        // sender on L2. Because this is checked here, we do not need to check again in
        // `finalizeWithdrawalTransaction`.
        require(
            _tx.target != address(this),
            "OptimismPortal: you cannot send messages to the portal contract"
        );

        // Get the output root and load onto the stack to prevent multiple mloads. This will
        // revert if there is no output root for the given block number.
        bytes32 outputRoot = L2_ORACLE.getL2Output(_l2OutputIndex).outputRoot;

        // Verify that the output root can be generated with the elements in the proof.
        require(
            outputRoot == Hashing.hashOutputRootProof(_outputRootProof),
            "OptimismPortal: invalid output root proof"
        );

Impact

Misleading spec

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol#L160-L182

Tool used

Manual Review

Recommendation

The specification should refer to _l2OutputIndex as one of the inputs instead of 'block number'.

@github-actions github-actions bot added the Specification An issue related to the specification (low severity) label Feb 20, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Feb 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Reward A payout will be made for this issue Specification An issue related to the specification (low severity)
Projects
None yet
Development

No branches or pull requests

1 participant