Liquid Staking

Summary

The _updateStrategyRewards function in the StakingPool contract allows malicious strategies to decrease the totalStaked amount and potentially drain pool funds. The contract should be updated to validate the strategy rewards and cap totalStaked before deployment.

In the _updateStrategyRewards function of StakingPool.sol, the contract sums up the rewards across all strategies being updated: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L530-L538

Then, it directly adds totalRewards to totalStaked: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L551-L553

If totalRewards is negative and its absolute value is greater than totalStaked, this will cause an integer underflow, setting totalStaked to a very large number.

Even if totalRewards is only slightly negative, it will still incorrectly decrease totalStaked when it should only ever stay the same or increase.

Vulnerability Details

This violates the expected behavior that updating rewards should never decrease the total staked.

The issue occurs because _updateStrategyRewards directly adds the totalRewards value returned by strategies to totalStaked: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L522-L600

If totalRewards is negative and its absolute value is greater than totalStaked, this will cause an integer underflow, setting totalStaked to a very large number. Even if totalRewards is only slightly negative, it will still incorrectly decrease totalStaked.

This breaks the core invariant that the total amount staked in the pool should always be the sum of all user stakes. If totalStaked decreases below that sum, the pool will not have enough funds to let everyone withdraw their original stake. Malicious strategies could exploit this bug to drain the pool's funds over time.

Some Scenarios

1. with a malicious strategy that returns negative rewards when updateDeposits is called.

2. Some users stake tokens into the pool so totalStaked > 0.

3. Call updateStrategyRewards with the malicious strategy, passing data that makes it return a negative reward greater in absolute value than the current totalStaked.

4. Observe that totalStaked decreases incorrectly, or underflows to a very large number.

Impact

This vulnerability allows malicious strategies to drain funds from the staking pool over time by repeatedly decreasing totalStaked with negative rewards. In the worst case, a single large negative reward could cause an integer underflow, setting totalStaked to a huge number and completely breaking the pool's accounting. This would block further withdrawals and reward distributions.

This undermines the core purpose of the staking pool, which is to hold all user funds and accurately distribute rewards.

Tools Used

Vs

Recommendations

Validate that the strategy rewards are non-negative, and cap the subtraction so totalStaked never goes below zero. Update the _updateStrategyRewards function as follows.