Update EmergencySystem.sol

[luis-immunefi]

No description provided.

[immunefi-magnus]

🛡️ Immunefi PR Reviews

We’ve assigned 2 code reviewer(s) to this PR.

They’ll begin the review shortly and leave feedback directly in the pull request.

[immunefi-magnus]

✅ AI Code Review Summary

I've completed reviewing this pull request using AI-powered analysis.

I found 2 issues that you may want to address. Please check the comments below for detailed explanations and suggested fixes.

📝 The AI may occasionally produce inaccurate or incomplete information. Please verify the suggestions before applying them.

This AI service is intended to assist, not replace professional expertise.

[immunefi-magnus]

Unrestricted Access to Emergency Shutdown Deactivation

Description:

TL;DR:
The deactivateEmergencyShutdown function in the EmergencySystem contract can be invoked by any external account due to the absence of an access control modifier, allowing unauthorized deactivation of the protocol's emergency shutdown.

The EmergencySystem contract lacks proper access control on its deactivateEmergencyShutdown function, allowing any address to reset the emergency shutdown state. This flaw compromises the emergency safety mechanisms by enabling unauthorized actors to resume protocol operations during critical incidents.

Details

In the EmergencySystem contract, the deactivateEmergencyShutdown function is defined as:

function deactivateEmergencyShutdown() external {
    emergencyShutdownActive = false;
    emit EmergencyShutdownDeactivated(msg.sender);
}

Unlike the activateEmergencyShutdown function which is guarded by the onlyOwner modifier, there is no similar restriction on deactivateEmergencyShutdown. As a result, any external caller can set the emergencyShutdownActive flag to false and generate an EmergencyShutdownDeactivated event, regardless of their authorization. This exposes the system to the risk of unauthorized protocol reactivation during emergency conditions, thereby undermining the intended isolation of sensitive protocol actions when the shutdown is active.

Impact

An attacker or any external user can exploit this vulnerability to prematurely disable the emergency shutdown control, potentially triggering the resumption of critical protocol operations when the system should still remain halted. This could lead to unauthorized transactions, compromise user funds, and disrupt protocol invariants, making it a significant risk to the overall protocol safety.

Mitigation Steps:
Restrict access to the deactivateEmergencyShutdown function by applying the onlyOwner modifier or an equivalent access control mechanism.
-Review and validate all functions managing critical state changes to ensure they enforce proper access restrictions.

• codexa

[immunefi-magnus]

Owner Assignment in Constructor Allows Zero Address, Permanently Disabling Owner Functions

Description:

TL;DR:
The EmergencySystem contract's constructor fails to validate the _owner parameter, allowing the zero address to be set as owner and thereby permanently disabling owner-only controls such as emergency shutdown.

In the EmergencySystem contract, the constructor calls _transferOwnership(_owner) without checking if _owner is not the zero address. This omission permits deployment of the contract with a zero address as the owner, making all functions guarded by the onlyOwner modifier inaccessible and resulting in permanent loss of control over administrative operations.

Details

The vulnerability is located in the constructor of the EmergencySystem contract defined in src/EmergencySystem.sol. The constructor directly passes the _owner parameter to _transferOwnership without validating that the provided address is valid.

constructor(address _owner) {
    // bypasses 2-step ownership transfer
    _transferOwnership(_owner);
}

If _owner is set to address(0), no valid owner ever exists. Consequently, any function that is restricted by the onlyOwner modifier, such as the activateEmergencyShutdown function, becomes permanently inaccessible. This issue is introduced at deployment and does not require further interactions to be exploited.

Impact

Deploying the contract with a zero address as the owner completely disables the administrative control functions, including the emergency shutdown mechanism. This disables the ability to respond to potential threats or operational issues, potentially leading to an inability to protect the system during critical situations. Although the vulnerability does not directly allow theft or state corruption, the loss of controlled access severely impacts the system's resilience and manageability in emergency scenarios.

Mitigation Steps:
Validate the _owner address in the constructor, ensuring that it is not the zero address (address(0)).
-Implement a requirement check, for example: require(_owner != address(0), "Invalid owner address");, before calling _transferOwnership(_owner).
-Consider adhering to safe ownership patterns such as the 2-step ownership transfer process to mitigate risks associated with incorrect ownership assignments.

• codexa