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

Loss of funds for removed operator due to missing `unbond` call

mbushbush

Title

Loss of funds for removed operator due to missing unbond call

Github link

https://github.com/Cyfrin/2024-09-stakelink/blob/main/contracts/linkStaking/base/VaultControllerStrategy.sol#L478

https://github.com/Cyfrin/2024-09-stakelink/blob/main/contracts/linkStaking/FundFlowController.sol#L414

https://github.com/Cyfrin/2024-09-stakelink/blob/main/contracts/linkStaking/OperatorVault.sol#L225

Summary

After an operator is removed, unbond should be called in order to withdraw principal amount of operator. But there's a flaw calling unbond which causes revert of unstakeRemovedPrincipal function for removed operators in exitVault.

Vulnerability Details

Ideally, operators are called unbound via updateVaultGroups function which loops through the vaults.

function updateVaultGroups(
uint256[] calldata _curGroupVaultsToUnbond,
uint256 _curGroupTotalDepositRoom,
uint256 _nextGroup,
uint256 _nextGroupTotalUnbonded
) external onlyFundFlowController {
for (uint256 i = 0; i < _curGroupVaultsToUnbond.length; ++i) {
-> vaults[_curGroupVaultsToUnbond[i]].unbond();
}
...

In the process, the vaults array are retrieved by FundFlowController._getTotalDepositRoom function which excludes removed operators.

function _getTotalDepositRoom(
address[] memory _vaults,
uint256 _numVaultGroups,
uint256 _vaultGroup,
uint256 _vaultMaxDeposits,
uint256 _depositIndex
) internal view returns (uint256, uint256[] memory) {
uint256 totalDepositRoom;
uint256 numNonEmptyVaults;
uint256[] memory nonEmptyVaults = new uint256[]();
​
for (uint256 i = _vaultGroup; i < _depositIndex; i += _numVaultGroups) {
-> if (IVault(_vaults[i]).isRemoved()) continue;
​
uint256 principalDeposits = IVault(_vaults[i]).getPrincipalDeposits();
totalDepositRoom += _vaultMaxDeposits - principalDeposits;
if (principalDeposits != 0) {
nonEmptyVaults[numNonEmptyVaults] = i;
numNonEmptyVaults++;
}
}
​
uint256[] memory nonEmptyVaultsFormatted = new uint256[]();
for (uint256 i = 0; i < numNonEmptyVaults; ++i) {
nonEmptyVaultsFormatted[i] = nonEmptyVaults[i];
}
​
return (totalDepositRoom, nonEmptyVaultsFormatted);
}

Hence, removed operators won't be called unbond function which means unbond period will never get started for them.
Finally, if it tries to exit via exitVault function, unstakeRemovedPrincipal will revert because of locked funds in Staking Contract.

Impact

Funds of removed operators get stuck in OperatorStakingPool contract.

Tools Used

Manual Review

Recommendations

There are several possible solutions to resolve this situation. One of them would be to adjust updateVaultGroups to properly unbond removed operators so that vaults can exit without issues.

Updates

Lead Judging Commences

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

Non-Grouped Vaults cannot be exited because there is no means to unbond them

Appeal created

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Non-Grouped Vaults cannot be exited because there is no means to unbond them

Support

FAQs

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