Submission Details
Severity:
Missing Access Control In `OperatorVCS::queueVaultRemoval` & `OperatorVCS::removeVault` Can Lead to Unwanted Removal of Vaults
Summary
The OperatorVCS::queueVaultRemoval function allows for vaults to be queued for removal. The OperatorVCS::removeVault function is the one that actually removes the vault from operation. However, in both of these functions there is no access control. No checks to see who is the address that is trying to queue or actually remove the vault.
Vulnerability Details
/**
* @notice Queues a vault for removal
* @dev a vault can only be queued for removal if the operator has been removed from the
* Chainlink staking contract
* @param _index index of vault
*/
function queueVaultRemoval(uint256 _index) external {
// @audit ?? Missing access control here? Anyone can queue to remove vault?
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]));
// update group accounting if vault is part of a group
if (_index < globalVaultState.depositIndex) {
uint256 group = _index % globalVaultState.numVaultGroups;
uint256[] memory groups = new uint256[]();
groups[0] = group;
fundFlowController.updateOperatorVaultGroupAccounting(groups);
// if possiible, remove vault right away
if (vaults[_index].claimPeriodActive()) {
removeVault(vaultsToRemove.length - 1);
}
}
}
/**
* @notice Queues a vault for removal
* @dev a vault can only be queued for removal if the operator has been removed from the
* Chainlink staking contract
* @param _index index of vault
*/
function queueVaultRemoval(uint256 _index) external {
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]));
// update group accounting if vault is part of a group
if (_index < globalVaultState.depositIndex) {
uint256 group = _index % globalVaultState.numVaultGroups;
uint256[] memory groups = new uint256[]();
groups[0] = group;
fundFlowController.updateOperatorVaultGroupAccounting(groups);
// if possiible, remove vault right away
if (vaults[_index].claimPeriodActive()) {
removeVault(vaultsToRemove.length - 1);
}
}
}
/**
* @notice Removes a vault that has been queued for removal
* @param _queueIndex index of vault in removal queue
*/
function removeVault(uint256 _queueIndex) public {
address vault = vaultsToRemove[_queueIndex];
vaultsToRemove[_queueIndex] = vaultsToRemove[vaultsToRemove.length - 1];
vaultsToRemove.pop();
_updateStrategyRewards();
(uint256 principalWithdrawn, uint256 rewardsWithdrawn) = IOperatorVault(vault).exitVault();
totalDeposits -= principalWithdrawn + rewardsWithdrawn;
totalPrincipalDeposits -= principalWithdrawn;
uint256 numVaults = vaults.length;
uint256 index;
for (uint256 i = 0; i < numVaults; ++i) {
if (address(vaults[i]) == vault) {
index = i;
break;
}
}
for (uint256 i = index; i < numVaults - 1; ++i) {
vaults[i] = vaults[i + 1];
}
vaults.pop();
token.safeTransfer(address(stakingPool), token.balanceOf(address(this)));
}
Impact
This could potentially let any actor to choose to remove a vault from operation, however it will not cause any loss of funds.
Tools Used
Manual Review, Solodit Checklist
Recommendations
Add access control modifiers or checks for msg.sender
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
2,53020 USDC / LOC
42,000 USDC
4,250 USDC
3,750 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.