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

Unclaimed Rewards Could Stuck During Community Strategy Removal in StakingPool

aksoy

Summary

The removeStrategy function in the StakingPool contract removes a strategy and withdraws its deposits. However, for community strategies, it does not claim rewards before removing the strategy. The unclaimed rewards remain stuck in the rewards controller or in strategy because the only way to claim and process them is by first calling CommunityVCS::claimRewards and then CommunityVCS::updateDeposits. However, after removal, updateDeposits can no longer be triggered, leading to rewards being permanently inaccessible.

Vulnerability Details

When a strategy is removed from the staking pool, the function _updateStrategyRewards is called to handle reward claims and deposit updates.

function removeStrategy(
uint256 _index,
bytes memory _strategyUpdateData,
bytes calldata _strategyWithdrawalData
) external onlyOwner {
require(_index < strategies.length, "Strategy does not exist");
​
uint256[] memory idxs = new uint256[]();
idxs[0] = _index;
_updateStrategyRewards(idxs, _strategyUpdateData);
​

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L308

Then _updateStrategyRewards is calling strategy.updateDepositsfor each strategy.

function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
// ... rest of the code
// sum up rewards and fees across strategies
for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
(
int256 depositChange,
address[] memory strategyReceivers,
uint256[] memory strategyFeeAmounts
) = strategy.updateDeposits(_data);
totalRewards += depositChange;

This works fine for operator strategy, but community strategy do not claim rewards during this process. Only way to claim fees for community strategy is calling CommunityVCS::claimRewards.

function claimRewards(
uint256[] calldata _vaults,
uint256 _minRewards
) external returns (uint256) {
address receiver = address(this);
uint256 balanceBefore = token.balanceOf(address(this));
for (uint256 i = 0; i < _vaults.length; ++i) {
ICommunityVault(address(vaults[_vaults[i]])).claimRewards(_minRewards, receiver);
}

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/CommunityVCS.sol#L122

Once claimed, fees need to be processed by strategy.updateDeposits to be transferred to the staking pool.

function updateDeposits(
bytes calldata
)
external
virtual
onlyStakingPool
returns (int256 depositChange, address[] memory receivers, uint256[] memory amounts)
{
depositChange = getDepositChange();

Since community strategies do not claim rewards as part of the updateDeposits function, any rewards not claimed before removal remain stuck. updateDeposits can only be triggered by the StakingPool contract, and once the strategy is removed, this function cannot be called anymore. Therefore, the unclaimed rewards are left in the rewards controller or in strategy contract are unrecoverable.

Impact

The rewards for the removed community strategy will be permanently stuck, as there is no function available to claim them after the strategy has been removed from the pool.

Tools Used

Manual

Recommendations

Ensure that the rewards for the community strategy are claimed before the strategy is removed. Modify the removeStrategy function to include a call to claimRewards for community strategies before the strategy is removed.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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