Liquid Staking

Summary

The StakingPool::removeStrategy function fails to execute successfully because it attempts to withdraw all deposited tokens tracked by the strategy, but the underlying withdrawal function reverts due to a condition in the VaultDepositController::withdraw function. As a result, the protocol owner is unable to remove strategies, which disrupts the protocol's functionality.

Vulnerability Details

When the StakingPool::removeStrategy function is called to remove a strategy, it first updates the strategy's rewards and fees, then attempts to withdraw all tokens deposited by the strategy. This amount is stored in a local variable totalStrategyDeposits, which represents the sum of all tokens staked on Chainlink by the strategy’s vaults. View relevant code here.

However, the function fails at the point of withdrawal due to the following condition in the VaultDepositController::withdraw function:

View this code section here.

The second condition, _amount > totalUnbonded, causes the function to revert because totalStrategyDeposits is always greater than or equal to totalUnbonded. This is due to the fact that totalUnbonded represents the number of tokens currently unbonded in the Chainlink staking contract, whereas totalStrategyDeposits represents the total tokens staked by the strategy. View relevant code for totalUnbonded.

Since totalUnbonded is a subset of totalStrategyDeposits, the function will always revert, preventing the strategy from being removed.

Impact

The protocol owner is unable to remove strategies from the staking pool. This creates operational issues, as unwanted strategies cannot be decommissioned, which negatively affects the overall functionality and adaptability of the protocol.

Tools Used

Manual

Recommendations

Modify the withdrawal logic to account for the difference between totalStrategyDeposits and totalUnbonded. This could involve a partial withdrawal mechanism or an allowance for withdrawing only the unbonded portion of the strategy’s deposits until all tokens are unbonded. This would prevent the reverts and allow the owner to remove strategies successfully.