bug(SuperchainConfig): extend() can re-activate an expired pause, bypassing Stage 1 requirement
- 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