Liquid Staking

Summary

checkUpkeep() function returns true when newRewards is less than 0.

Vulnerability Details

The checkUpkeep function in LSTRewardSplitter incorrectly returns true when the calculated rewards are negative.

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L89C4-L96C6

The function returns true when newRewards < 0, which is incorrect.

In performUpkeep:

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L101C5-L110C6

When newRewards is negative, the contract actually increases principalDeposits. This is because:

1. -1 * newRewards converts the negative value to positive.

2. This positive value is cast to uint256.

3. principalDeposits is decreased by this positive value, which is mathematically equivalent to increasing it.

If this situation occurs multiple times, it could lead to a significant overstatement of the principal amount.

Impact

When there's a loss (i.e., the current balance is less than the recorded principal), the contract incorrectly inflates the principal instead of acknowledging the loss.

Future reward calculations based on this inflated principal will be incorrect, potentially leading to under-distribution of actual rewards.

Tools Used

Manual review

Recommendations

The performUpkeep function should be modified to correctly handle negative reward scenarios and not modify principalDeposits to a number gotten from the thin air.