Liquid Staking

Summary

The _updateStrategyRewards function in the StakingPool contract has a vulnerability that allows the total distributed fees to exceed the intended limit relative to the total staked amount. This issue arises from performing the safety check after updating totalFeeAmounts, which can lead to totalStaked being incorrectly incremented.

The bug occurs in the _updateStrategyRewards function of StakingPool.sol. Here's how it happens:

1. The function calculates the total rewards earned by the strategies since the last update.

2. If there are positive rewards (totalRewards > 0), it proceeds to calculate the fees to be distributed to the fee receivers.

3. For each fee receiver, it calculates the fee amount based on the totalRewards and the fee's basisPoints, and adds it to the totalFeeAmounts.

4. After calculating all the fees, there is a safety check: if (totalFeeAmounts >= totalStaked) { totalFeeAmounts = 0; }. This is intended to prevent the total fees from exceeding the total staked amount.

5. However, the issue is that this safety check happens after the fees have already been added to totalFeeAmounts. If the sum of the calculated fees exceeds totalStaked, totalStaked will still be incremented by that excessive totalFeeAmounts before the safety check sets totalFeeAmounts to 0.

This means that in a scenario where the total fees calculated based on totalRewards and fees[i].basisPoints exceed the totalStaked amount, totalStaked will be incorrectly incremented, violating the intended limit on fee distribution.

Vulnerability Details

The vulnerability lies in the order of operations: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L550-L579

1. Fees are added to totalFeeAmounts in the loop.

2. The safety check to prevent excessive fees is performed after the loop, when totalFeeAmounts may have already exceeded totalStaked.

3. If totalFeeAmounts > 0, shares are minted based on the potentially excessive totalFeeAmounts.

This sequence allows for the incorrect incrementing of totalStaked and minting of shares when the total fees exceed the intended limit.

The order of operations when calculating and distributing fees. The function first calculates the total rewards earned by the strategies since the last update. If there are positive rewards, it proceeds to calculate the fees for each fee receiver based on their basisPoints and adds them to the totalFeeAmounts.

However, the safety check to prevent the total fees from exceeding the total staked amount is performed after the fees have already been added to totalFeeAmounts. If the sum of the calculated fees exceeds totalStaked, totalStaked will still be incremented by that excessive totalFeeAmounts before the safety check sets totalFeeAmounts to 0.

This means that in a scenario where the total fees calculated based on totalRewards and fees[i].basisPoints exceed the totalStaked amount, totalStaked will be incorrectly incremented, violating the intended limit on fee distribution.

Consider the following scenario:

1. The staking pool has strategies with rewards since the last update, resulting in totalRewards > 0.

2. The staking pool has fee receivers set up with basisPoints such that the sum of (totalRewards * fees[i].basisPoints) / 10000 for all fees exceeds the current totalStaked.

When _updateStrategyRewards is called in this scenario, it will incorrectly increment totalStaked by the excessive totalFeeAmounts, and the safety check will fail to prevent this.

Impact

This bug affects users because it allows the staking pool to distribute more fees than intended, potentially draining the pool's staked balance over time if the situation persists. It breaks the expected limit on fee distribution relative to the total staked amount.

Tools Used

Vs

Recommendations

The safety check should be performed before adding the calculated fees to totalFeeAmounts and updating the pool's state. This ensures that totalStaked is only incremented if the total fees are within the allowed limit, preventing the incorrect distribution of excessive fees.