075.md

csanuragjain

medium

Challenger can override the 7 day finalization period

Summary

All withdrawals are finalized after a 7 days window (finalization period). After this duration transaction are confirmed and user can surely withdraw their balance. But due to lack of check, challenger can delete a l2Output which is older than 7 days meaning withdrawals will stop working for even confirmed transaction

Vulnerability Detail

1. Proposer has proposed L2 output for a _l2BlockNumber which creates entries on l2Outputs using the proposeL2Output. Assume this creates a new l2Output at index X

l2Outputs.push(
            Types.OutputProposal({
                outputRoot: _outputRoot,
                timestamp: uint128(block.timestamp),
                l2BlockNumber: uint128(_l2BlockNumber)
            })
        );

2. proveWithdrawalTransaction has been called for user linked to this l2Output

3. Finalization period(7 day) is over after proposal and Users is ready to call finalizeWithdrawalTransaction to withdraw their funds

4. Since confirmation is done, User A is sure that he will be able to withdraw and thinks to do it after coming back from his holidays

5. Challenger tries to delete the index X (Step 1), ideally it should not be allowed as already confirmed. But since there is no such timeline check so the l2Output gets deleted

function deleteL2Outputs(uint256 _l2OutputIndex) external {
        require(
            msg.sender == CHALLENGER,
            "L2OutputOracle: only the challenger address can delete outputs"
        );

        // Make sure we're not *increasing* the length of the array.
        require(
            _l2OutputIndex < l2Outputs.length,
            "L2OutputOracle: cannot delete outputs after the latest output index"
        );

        uint256 prevNextL2OutputIndex = nextOutputIndex();

        // Use assembly to delete the array elements because Solidity doesn't allow it.
        assembly {
            sstore(l2Outputs.slot, _l2OutputIndex)
        }

        emit OutputsDeleted(prevNextL2OutputIndex, _l2OutputIndex);
    }

6. User comes back and now tries to withdraw but the withdraw fails since the l2Output index X does not exist anymore. This is incorrect and nullifies the network guarantee.

Note: In case of a separate output root could be proven then user withdrawal will permanently stuck. Ideally if such anomaly could not be caught within finalization period then user should be allowed to withdraw

Impact

Withdrawal will fail for confirmed transaction

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/packages/contracts-bedrock/contracts/L1/L2OutputOracle.sol#L128-L148

Tool used

Manual Review

Recommendation

Add below check in

require(getL2Output(_l2OutputIndex).timestamp<=FINALIZATION_PERIOD_SECONDS, "Output already confirmed");