Liquid Staking

Summary

removeStrategy() function in StakingPool contract fails to update the totalStaked variable when withdrawing tokens from a removed strategy. This leads to an inconsistency between totalStaked and the actual total staked balance, affecting reward calculations, distributions, and reporting.

This bug affects users because it leads to an incorrect accounting of the total staked tokens. As a result:

1. The totalStaked variable will not accurately represent the total amount of tokens staked in the pool and its strategies.

2. Functions that rely on totalStaked, such as _totalStaked() and getStrategyRewards(), will return incorrect values.

Vulnerability Details

When a strategy is removed, the function withdraws all the tokens from the strategy using strategy.withdraw(totalStrategyDeposits, _strategyWithdrawalData). However, after the withdrawal, the totalStaked variable is not updated to reflect the increase in the pool's balance. As a result, totalStaked will be less than the actual total amount of staked tokens.

This bug affects the accuracy of the totalStaked variable and any functions that rely on it, such as _totalStaked() and getStrategyRewards(). Incorrect totalStaked values can lead to incorrect reward calculations, distributions, and reporting of the pool's total staked balance. https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L299-L321

The vulnerability lies in the removeStrategy() function, specifically in the lines: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L312-L314

After withdrawing the tokens from the strategy, the totalStaked variable is not updated to account for the increase in the pool's balance. This leads to an inconsistency between totalStaked and the actual total staked balance.

Here's how the bug happens:

1. When removeStrategy() is called, it first updates the rewards for the strategy being removed by calling _updateStrategyRewards().

2. Then, it checks if the strategy has any deposits by calling strategy.getTotalDeposits(). If there are deposits (totalStrategyDeposits > 0), it withdraws all the tokens from the strategy using strategy.withdraw(totalStrategyDeposits, _strategyWithdrawalData).

3. However, after withdrawing the tokens from the strategy, the totalStaked variable is not updated to reflect the increase in the pool's balance. This means that totalStaked will be less than the actual total amount of staked tokens.

Impact

Incorrect reward calculations and distributions based on the totalStaked value.

Tools Used

Vs

Recommendations

Update the totalStaked variable after withdrawing tokens from the strategy in the removeStrategy() function by updating totalStaked with the withdrawn amount, the contract will maintain an accurate representation of the total staked tokens.