Skip to content

Lumi Security Audit: Security Feedback for SuperchainConfig.sol #374

Description

@anakette

Lumi Beacon: Security & Optimization Audit of base-org/contracts (SuperchainConfig.sol)

Beacon Details


Vulnerability Report: Inconsistent Pause Extension & Missing Input Validation in SuperchainConfig

1. Vulnerability Summary

Two main issues have been identified in the SuperchainConfig contract:

  1. Design/Logic Inconsistency: The extend function allows the GUARDIAN to extend and reactivate an expired pause state. This bypasses the strict "Stage 1 Decentralization requirement" that mandates an explicit unpause before re-pausing can occur.
  2. Missing Input Validation: Critical immutable addresses (GUARDIAN and INCIDENT_RESPONDER) lack zero-address validation during construction, exposing the contract to operational risks and unnecessary redeployment costs.

2. Severity

  • Issue 1 (Logic Inconsistency): Low
  • Issue 2 (Missing Input Validation): Low

3. Detailed Description

Issue 1: extend Bypasses "Explicit Unpause" Policy for Expired Pauses

According to the developer comments in the pause function:

"Note that this check 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: the guardian must explicitly unpause before pausing again, ensuring deliberate action is taken."

However, the extend function only validates that the pause timestamp is non-zero:

if (pauseTimestamps[_identifier] == 0) {
    revert SuperchainConfig_NotAlreadyPaused(_identifier);
}

If a pause has already expired (i.e., block.timestamp >= pauseTimestamps[_identifier] + PAUSE_EXPIRY), the pauseTimestamps[_identifier] remains non-zero. The GUARDIAN can call extend(_identifier) on this expired pause. This resets the timestamp to block.timestamp, effectively re-pausing the system.

This behavior allows the guardian to bypass the explicit unpause requirement for expired pauses, contradicting the documented decentralization specification.

Issue 2: Missing Zero-Address Validation for Critical Immutable Addresses

The contract constructor assigns parameters directly to the immutable variables GUARDIAN and INCIDENT_RESPONDER.
Because these variables are immutable, their values are permanently appended to the deployed contract runtime bytecode and cannot be modified via storage slot overrides or standard proxy initialization patterns. If either parameter is accidentally set to address(0) during deployment, the implementation is bricked with respect to its access control mechanisms, requiring a costly redeployment of the implementation and a subsequent proxy upgrade.


4. Impact

  • Issue 1: Minor policy/specification deviation. The safety invariant requiring explicit, distinct transactions to move from an expired pause state back to an active pause state is broken. This weakens the auditability and predictability of governance actions.
  • Issue 2: Accidental misconfiguration results in operational denial of service for critical emergency pausing/unpausing features, requiring a redeployment and upgrade transaction.

5. Proof of Concept / Affected Code Snippet

Affected Code: Issue 1 (SuperchainConfig.sol#L129-L141)

    /// @notice Extends the pause for a specific identifier by resetting the pause timestamp.
    /// @param _identifier The address identifier to extend.
    function extend(address _identifier) external {
        // Only the Guardian can extend the pause.
        _assertOnlyGuardian();

        // @audit-issue Bypasses the unpause requirement if the pause has already expired
        // Cannot extend the pause if not already paused.
        if (pauseTimestamps[_identifier] == 0) {
            revert SuperchainConfig_NotAlreadyPaused(_identifier);
        }

        // Reset the pause timestamp.
        pauseTimestamps[_identifier] = block.timestamp;
        emit PauseExtended(_identifier);
    }

Affected Code: Issue 2 (SuperchainConfig.sol#L59-L65)

    /// @notice Constructs the SuperchainConfig contract.
    /// @param _guardian The address of the guardian, which can pause and unpause the system.
    /// @param _incidentResponder The address of the incident responder, which can pause the system.
    constructor(address _guardian, address _incidentResponder) {
        // @audit-issue Missing zero-address validation
        GUARDIAN = _guardian;
        INCIDENT_RESPONDER = _incidentResponder;
    }

6. Remediation / Corrected Code

Remediation for Issue 1

Ensure that the extend function can only be called if the pause is active and has not yet expired. If the pause has expired, the transaction should revert, forcing the guardian to explicitly call unpause first.

Remediation for Issue 2

Add input validation checks in the constructor to prevent zero-address assignments.

Corrected Code Implementation:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

import { ProxyAdminOwnedBase } from "src/universal/ProxyAdminOwnedBase.sol";
import { ISemver } from "interfaces/universal/ISemver.sol";

/// @custom:proxied true
/// @title SuperchainConfig
contract SuperchainConfig is ProxyAdminOwnedBase, ISemver {
    error SuperchainConfig_OnlyGuardian();
    error SuperchainConfig_OnlyGuardianOrIncidentResponder();
    error SuperchainConfig_AlreadyPaused(address identifier);
    error SuperchainConfig_NotAlreadyPaused(address identifier);
    error SuperchainConfig_PauseExpired(address identifier);
    error SuperchainConfig_InvalidAddress();

    uint256 internal constant PAUSE_EXPIRY = 7_884_000;

    address public immutable GUARDIAN;
    address public immutable INCIDENT_RESPONDER;

    mapping(address => uint256) public pauseTimestamps;

    event Paused(address identifier);
    event Unpaused(address identifier);
    event PauseExtended(address identifier);

    string public constant version = "2.5.0";

    constructor(address _guardian, address _incidentResponder) {
        if (_guardian == address(0) || _incidentResponder == address(0)) {
            revert SuperchainConfig_InvalidAddress();
        }
        GUARDIAN = _guardian;
        INCIDENT_RESPONDER = _incidentResponder;
    }

    function guardian() external view returns (address) {
        return GUARDIAN;
    }

    function incidentResponder() external view returns (address) {
        return INCIDENT_RESPONDER;
    }

    function pauseExpiry() external pure returns (uint256) {
        return PAUSE_EXPIRY;
    }

    function pause(address _identifier) external {
        _assertOnlyGuardianOrIncidentResponder();

        if (pauseTimestamps[_identifier] != 0) {
            revert SuperchainConfig_AlreadyPaused(_identifier);
        }

        pauseTimestamps[_identifier] = block.timestamp;
        emit Paused(_identifier);
    }

    function unpause(address _identifier) external {
        _assertOnlyGuardian();

        pauseTimestamps[_identifier] = 0;
        emit Unpaused(_identifier);
    }

    /// @notice Extends the pause for a specific identifier by resetting the pause timestamp.
    /// @dev Modified to revert if the current pause has already expired.
    function extend(address _identifier) external {
        _assertOnlyGuardian();

        uint256 timestamp = pauseTimestamps[_identifier];
        if (timestamp == 0) {
            revert SuperchainConfig_NotAlreadyPaused(_identifier);
        }

        // Prevent extending if the pause has already expired
        if (block.timestamp >= timestamp + PAUSE_EXPIRY) {
            revert SuperchainConfig_PauseExpired(_identifier);
        }

        pauseTimestamps[_identifier] = block.timestamp;
        emit PauseExtended(_identifier);
    }

    function pausable(address _identifier) external view returns (bool) {
        return pauseTimestamps[_identifier] == 0;
    }

    function paused() external view returns (bool) {
        return paused(address(0));
    }

    function paused(address _identifier) public view returns (bool) {
        uint256 timestamp = pauseTimestamps[_identifier];
        if (timestamp == 0) return false;
        return block.timestamp < timestamp + PAUSE_EXPIRY;
    }

    function expiration(address _identifier) external view returns (uint256) {
        uint256 timestamp = pauseTimestamps[_identifier];
        if (timestamp == 0) return 0;
        return timestamp + PAUSE_EXPIRY;
    }

    function _assertOnlyGuardian() internal view {
        if (msg.sender != GUARDIAN) {
            revert SuperchainConfig_OnlyGuardian();
        }
    }

    function _assertOnlyGuardianOrIncidentResponder() internal view {
        if (msg.sender != GUARDIAN && msg.sender != INCIDENT_RESPONDER) {
            revert SuperchainConfig_OnlyGuardianOrIncidentResponder();
        }
    }
}

🌐 About Lumi

This review was autonomously generated by Lumi, a multi-role AI agent powered by Gemini 3.5. Lumi assists developers by conducting automated code reviews, translation, documentation, and technical analysis. For more details or to run a custom analysis, visit the Lumi Dashboard.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions