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

`removeVault` does not update `vaultMapping`

bladesec

Summary

In OperatorVCS contract, vaultMapping represents if a vault is active or not.
However, when a vault is removed through removeVault, it does not update the mapping status, as a result, the vault is regarded active even after it's removed.

Vulnerability Details

The vaultMapping in OperatorVCS contract represents if a vault is active or not.
Logically, when a vault is added it should be set to true, and when a vault is removed, it should be set to false.
But removeVault does not set the mapping, which keeps the removed vault still active.

Here's the main issue that can be caused by removed vault being active:

function withdrawOperatorRewards(
address _receiver,
uint256 _amount
) external returns (uint256) {
if (!vaultMapping[msg.sender]) revert SenderNotAuthorized();
​
IERC20Upgradeable lsdToken = IERC20Upgradeable(address(stakingPool));
uint256 withdrawableRewards = lsdToken.balanceOf(address(this));
uint256 amountToWithdraw = _amount > withdrawableRewards ? withdrawableRewards : _amount;
​
unclaimedOperatorRewards -= amountToWithdraw;
lsdToken.safeTransfer(_receiver, amountToWithdraw);
​
return amountToWithdraw;
}

As shown in the above code snippet, withdrawOperatorRewards function is only called by active vault to withdraw rewards.
Since vaultMapping for the removed vault remains true, the removed vault can call this function to potentially withdraw assets from the VCS.

Impact

Unauthorized access to withdrawOperatorRewards function by removed vault and potentially steal rewards from the VCS.

Tools Used

Manual Review

Recommendations

When removeVault is called, it should set vaultMapping to false.

function removeVault(uint256 _queueIndex) public {
address vault = vaultsToRemove[_queueIndex];
​
+ vaultMapping[vault] = false;
​
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)));
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`removeVault` does not update `vaultMapping`

Support

FAQs

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