bug(SuperchainConfig): extend() can re-activate an expired pause, bypassing Stage 1 requirement

[Sertug17]

Summary

SuperchainConfig.extend() allows the guardian to re-activate an expired pause by resetting its timestamp, bypassing the Stage 1 Decentralization requirement that the guardian must explicitly unpause() before pause() can be called again.

Root Cause

pause() correctly enforces the invariant (lines 91–96):

// "intentionally prevents re-pausing even after a pause has expired
// (when paused() returns false but the timestamp is still non-zero).
// This is a Stage 1 Decentralization requirement."
if (pauseTimestamps[_identifier] != 0) revert SuperchainConfig_AlreadyPaused(_identifier);

But extend() uses a weaker check (lines 122–125):

// Only reverts when timestamp == 0 (never paused), NOT when expired
if (pauseTimestamps[_identifier] == 0) revert SuperchainConfig_NotAlreadyPaused(_identifier);
pauseTimestamps[_identifier] = block.timestamp; // re-activates expired pause!

An expired pause has pauseTimestamps[id] != 0 AND paused(id) == false. It passes the extend() check but would correctly revert pause().

Attack Scenario

1. Guardian calls pause(id) → pauseTimestamps[id] = T
2. 3 months pass → paused(id) returns false (expired), but pauseTimestamps[id] = T ≠ 0
3. pause(id) → reverts ✅ (Stage 1 enforced)
4. extend(id) → succeeds ❌ → pauseTimestamps[id] = now → system re-paused without the required unpause cycle

Missing Test

No test covers extend() called after a pause expires:

function test_extend_expiredPause_shouldRevert() external {
    _pauseAsGuardian(address(this));
    vm.warp(block.timestamp + PAUSE_EXPIRY + 1);

    // paused() is false — pause has expired
    assertFalse(superchainConfig.paused(address(this)));

    // extend() should revert here but currently succeeds
    vm.prank(superchainConfig.guardian());
    vm.expectRevert(
        abi.encodeWithSelector(ISuperchainConfig.SuperchainConfig_NotAlreadyPaused.selector, address(this))
    );
    superchainConfig.extend(address(this)); // BUG: does not revert
}

Fix

function extend(address _identifier) external {
    _assertOnlyGuardian();
    // Use paused() instead of raw timestamp check to catch expired pauses
    if (!paused(_identifier)) {
        revert SuperchainConfig_NotAlreadyPaused(_identifier);
    }
    pauseTimestamps[_identifier] = block.timestamp;
    emit PauseExtended(_identifier);
}