Update VaultFreezer.sol - #14
Conversation
🛡️ Immunefi PR ReviewsWe’ve assigned 10 code reviewer(s) to this PR. They’ll begin the review shortly and leave feedback directly in the pull request. |
✅ AI Code Review SummaryI'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.
This AI service is intended to assist, not replace professional expertise. |
There was a problem hiding this comment.
Unrestricted freezeVault Enables Arbitrary Vault Freezing Without Access Control
Description:
TL;DR:
The freezeVault function in the VaultFreezer contract lacks access control, allowing any address to mark any vault as frozen, potentially enabling a denial-of-service in future protocol logic.
In the VaultFreezer contract, the freezeVault function (at src/VaultFreezer.sol:38) can be called by any address because it lacks role-based restrictions. This vulnerability permits arbitrary freezing of vaults, which may lead to operational disruptions if downstream functions enforce the isFrozen flag.
Details
The freezeVault function is defined without any access control checks. Unlike the unfreezeVault function, which is restricted using the onlyRole(FREEZER_ROLE) modifier, freezeVault sets the state variable isFrozen to true for a given vault address without verifying the caller's authorization. The relevant code snippet is as follows:
function freezeVault(address vault) external {
isFrozen[vault] = true;
emit VaultFreezed(vault);
}The absence of a role check means that any externally owned account or contract can freeze any vault. Although the current implementation does not enforce additional behavior based on the isFrozen flag, it is noted in the protocol documentation and code context that this flag may be used downstream to restrict vault operations such as withdrawals, rewards distribution, or administrative changes. This leads to a potential denial-of-service (DoS) situation in the future if the isFrozen flag is utilized to gate critical functionalities.
Impact
If exploited, an attacker could arbitrarily freeze vaults, preventing legitimate operations such as withdrawals, rewards claims, or administrative updates. This vulnerability may lead to a partial or complete denial-of-service against the protocol, resulting in potential lock-up of user funds and disruption of protocol operations, should downstream logic enforce restrictions based on the isFrozen flag.
Mitigation Steps:
Restrict the freezeVault function with an appropriate access control modifier (e.g., onlyRole(FREEZER_ROLE)) to ensure that only authorized addresses can freeze vaults.
-Conduct a thorough review of the downstream logic where the isFrozen flag is used and ensure that its enforcement aligns with the intended protocol security model.
- png_wireless_6n8x3o
There was a problem hiding this comment.
Freeze state is unenforced, allowing vault operations despite freezing
Description:
TL;DR:
The freeze mechanism implemented via the VaultFreezer contract is ineffective because downstream vault operations do not check the isFrozen state before executing critical functions.
In the VaultFreezer contract, while the unfreezeVault function manages the isFrozen state for vault addresses, there is a lack of validation in the downstream vault logic. This allows operations such as withdrawals, reward claims, or administrative actions to proceed even when a vault is marked as frozen, compromising protocol invariants.
Details
The vulnerability originates from the absence of freeze-state checks in the functions that manage vault operations. Although the VaultFreezer contract includes functions like freezeVault and unfreezeVault which modify the isFrozen mapping and emit events (VaultFreezed and VaultUnfreezed respectively), no subsequent logic in the vault's operations verifies this state. For example, critical functions that process user withdrawals or calculate rewards do not include a condition such as require(!isFrozen[vault], 'Vault is frozen'). As a result, even if a vault is marked as frozen to prevent further operations, the intended safety measure is bypassed since these checks are not enforced in any downstream call logic.
Impact
By allowing operations to proceed on a vault that should be frozen, an attacker or misconfigured system may trigger actions such as unauthorized fund withdrawals, alteration of vault states, or reward distributions. This can lead to direct financial losses, breaches of protocol invariants, and potential protocol corruption.
Mitigation Steps:
Integrate a freeze state check using the isFrozen mapping in all critical vault functions to block operations when a vault is marked as frozen.
-Implement and enforce a condition, such as require(!isFrozen[vault], 'Vault is frozen'), in functions that handle withdrawals, reward distributions, and administrative modifications.
-Perform a thorough audit of the call-graph to ensure that every function intended to be gated by the freeze mechanism correctly validates the vault's state.
- png_wireless_6n8x3o
No description provided.