base / base/contracts

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

Open Beginner friendly
#391 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Solidity
Stars
325
Forks
245
Avg merge
13h 48m
Merged PRs (30d)
18

Description

## 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):

```solidity
// "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):

```solidity
// 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:

```solidity
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

```solidity
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);
}
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with SuperchainConfig.extend(), pause(), and paused(), then inspect the existing SuperchainConfig tests around guardian pauses and PAUSE_EXPIRY. Add coverage for extending an expired pause and verify it reverts with SuperchainConfig_NotAlreadyPaused; done means expired pauses cannot be reactivated through extend().

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity
Domain
blockchain, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.