0xcoinymous - Inability to Challenge Malicious Batches After Protocol Pause Exceeds ChallengeWindow

[sherlock-admin2]

0xcoinymous

High

Inability to Challenge Malicious Batches After Protocol Pause Exceeds ChallengeWindow

Summary

There is a vulnerability in the optimistic rollup contract that prevents sequencers from challenging malicious batches when the protocol pause time exceeds the ChallengeWindow.

Consider the following scenarios:

1. A sequencer submits a batch, and before it can be challenged, the protocol is paused.
2. A challenger initiates a challenge, but before calling proveState, the protocol is paused.

In both cases, the protocol pause leads to the deletion of challenges[batchChallenged]. Since there is no limit on how long the protocol can remain paused, if the pause duration exceeds the ChallengeWindow, the sequencers are no longer able to challenge the batch after unpausing. Worse, anyone can call the public finalizeBatch function to finalize the malicious batch, allowing invalid state transitions to be confirmed.

Proof of Concept (PoC)

Assume the previous batches have been finalized, and a sequencer submits a new batch. Challengers have until finalizationPeriodSeconds after the batch submission to challenge the batch. The finalizationPeriodSeconds is set during the batch submission as shown below through commitBatch function:

    batchDataStore[_batchIndex] = BatchData(
        block.timestamp,
@>      block.timestamp + finalizationPeriodSeconds,
        _loadL2BlockNumber(batchDataInput.chunks[_chunksLength - 1]),
        IL1Staking(l1StakingContract).getStakerBitmap(_msgSender())
    );

If the protocol is paused due to governance decisions, network issues, or any other reason before a challenge is completed or even initiated, the following code is executed within the setPause function:

function setPause(bool _status) external onlyOwner {
    if (_status) {
        _pause();
        // if challenge exist and not finished yet, return challenge deposit to challenger
        if (inChallenge) {
            batchChallengeReward[challenges[batchChallenged].challenger] += challenges[batchChallenged].challengeDeposit;
@>          delete challenges[batchChallenged];
@>          inChallenge = false;
        }
    } else {
        _unpause();
    }
}

In this code:

• Pausing the protocol deletes the entire challenges[batchChallenged] data.
• The inChallenge flag is set to false.

If the pause exceeds the finalizationPeriodSeconds, unpausing the protocol would make it impossible for anyone to challenge the batch. This happens because the challengeState function checks whether the batch is still within the ChallengeWindow:

function challengeState(uint64 batchIndex) external payable onlyChallenger nonReqRevert whenNotPaused {
        require(!inChallenge, "already in challenge");
        require(lastFinalizedBatchIndex < batchIndex, "batch already finalized");
        require(committedBatches[batchIndex] != 0, "batch not exist");
        require(challenges[batchIndex].challenger == address(0), "batch already challenged");
        // check challenge window
@>      require(batchInsideChallengeWindow(batchIndex), "cannot challenge batch outside the challenge window");
    ...
}

After unpausing, because the batch is no longer within the ChallengeWindow, this check fails and prevents anyone from challenging the batch.

Additionally, since the finalizeBatch function is public, anyone can call it to finalize the batch without restriction, allowing the malicious batch to be confirmed:

function finalizeBatch(bytes calldata _batchHeader) public nonReqRevert whenNotPaused {
    (uint256 memPtr, bytes32 _batchHash) = _loadBatchHeader(_batchHeader);
    uint256 _batchIndex = BatchHeaderCodecV0.getBatchIndex(memPtr);

    require(committedBatches[_batchIndex] == _batchHash, "incorrect batch hash");
    require(batchExist(_batchIndex), "batch not exist");
    require(!batchInChallenge(_batchIndex), "batch in challenge");
    require(!batchChallengedSuccess(_batchIndex), "batch should be reverted");
    require(!batchInsideChallengeWindow(_batchIndex), "batch in challenge window");
    // Check and update last finalized batch
    require(finalizedStateRoots[_batchIndex - 1] == BatchHeaderCodecV0.getPrevStateHash(memPtr), "incorrect previous state root");
    unchecked {
        require(lastFinalizedBatchIndex + 1 == _batchIndex, "incorrect batch index");
        lastFinalizedBatchIndex = _batchIndex;
    }
    ...
}

Important Note: If, before pausing the protocol, another sequencer has already challenged a batch and there is an active challenge, this bug remains valid because the setPause function deletes the entire challenges[batchChallenged]. As a result, the challenge is lost even though it was active before the pause. Therefore, additional time should be provided to challengers when the protocol is paused to ensure fair continuation of the challenge process.

Tool used

Manual Review

Recommendation:

To address the issue, the pause mechanism should be implemented more carefully. The key improvement is to account for the time the protocol spends in a paused state and extend the ChallengeWindow accordingly.

uint pausedTime;
function setPause(bool _status) external onlyOwner {
    if (_status) {
        _pause();
        if (inChallenge) {
            batchChallengeReward[challenges[batchChallenged].challenger] += challenges[batchChallenged]
                .challengeDeposit;
            delete challenges[batchChallenged];
            inChallenge = false;
        }
        pausedTime = block.timestamp;
    } else {
        _unpause();
        for (uint256 i = lastFinalizedBatchIndex + 1; i <= lastCommittedBatchIndex; i++) {
            batchDataStore[i].finalizeTimestamp += (block.timestamp - pausedTime);
        }
    }
}

this bug poses a significant risk to the integrity of the optimistic rollup protocol. If not addressed, malicious actors could bypass the challenge process during protocol pauses, leading to the finalization of invalid batches. By implementing the proposed solution, we can ensure that challengers still have a fair opportunity to dispute batches even after long pauses, safeguarding the protocol's security.