Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Lack of Access Control in queueVaultRemoval and removeVault Functions

mansa11

Summary

The queueVaultRemoval and removeVault functions in the OperatorVCS contract lack proper access control, allowing any external actor to queue vaults for removal and remove vaults from the system.

Details

The queueVaultRemoval function (lines 276-297) and the removeVault function (lines 303-330) in the OperatorVCS contract are both declared as external and public respectively, without any access control modifiers. This means that any external account can call these functions, potentially disrupting the intended operation of the vault management system.

The queueVaultRemoval function allows adding a vault to the removal queue, while the removeVault function actually removes the vault from the system, updates accounting, and transfers tokens. These are critical operations that should be restricted to authorized roles, such as the contract owner or a designated manager.

Impact

The lack of access control on these functions could lead to several severe issues:

  1. Unauthorized removal of active vaults, disrupting staking operations.

  2. Manipulation of the vault removal queue, potentially leading to incorrect accounting.

  3. Premature removal of vaults that are not ready for removal, causing loss of funds or rewards.

  4. Potential denial of service by repeatedly queueing and removing vaults.

These vulnerabilities could result in significant financial losses, disruption of the staking system, and loss of user trust.

Code Snippet

function queueVaultRemoval(uint256 _index) external { //@audit no aces control
address vault = address(vaults[_index]);
​
if (!IVault(vault).isRemoved()) revert OperatorNotRemoved();
for (uint256 i = 0; i < vaultsToRemove.length; ++i) {
if (vaultsToRemove[i] == vault) revert VaultRemovalAlreadyQueued();
}
​
vaultsToRemove.push(address(vaults[_index]));
​
// ... (rest of the function)
}
​
function removeVault(uint256 _queueIndex) public { //@audit no aces control
address vault = vaultsToRemove[_queueIndex];
​
vaultsToRemove[_queueIndex] = vaultsToRemove[vaultsToRemove.length - 1];
vaultsToRemove.pop();
​
_updateStrategyRewards();
(uint256 principalWithdrawn, uint256 rewardsWithdrawn) = IOperatorVault(vault).exitVault();
​
// ... (rest of the function)
}

Recommendation

Implement proper access control on both functions to restrict their usage to authorized roles only. This can be achieved by:

Adding an onlyOwner or similar access control modifier to both functions.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.