Liquid Staking

Summary

The LSTRewardsSplitter contract has a bug in the _splitRewards function that causes the principalDeposits state variable to be incorrectly updated. This leads to a loss of principal for users and breaks the contract's accounting.

Vulnerability Details

_splitRewards function incorrectly updates the principalDeposits state variable after transferring out the accumulated rewards. This leads to a loss of principal for users and breaks the contract's accounting.

The principalDeposits variable is intended to track the total amount of LST tokens deposited by users, separate from any rewards earned. However, the _splitRewards function, which is called by performUpkeep and splitRewards, modifies principalDeposits by setting it to the current LST balance of the contract after distributing the rewards: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/lstRewardsSplitter/LSTRewardsSplitter.sol#L173-L187

This is incorrect because principalDeposits should only be updated when users deposit or withdraw tokens, not when rewards are paid out. By setting it to the balance after rewards are distributed, the contract loses track of the original deposit amounts and treats some of the principal as if it were paid out as rewards.

Impact

1. Users may be unable to withdraw their full deposit amounts, as the withdraw function checks against the incorrect principalDeposits value, which may be lower than the actual deposits.

2. Users may lose a portion of their principal deposits when rewards are distributed, as the contract incorrectly treats some of the principal as paid-out rewards.

Tools Used

Manual Review

Recommendations

Tthe line that updates principalDeposits should be removed from the _splitRewards function.

The deposit and withdraw functions already correctly manage the principalDeposits variable by incrementing and decrementing it as needed. Modifying it based on the balance after rewards are paid out introduces the bug.

By removing the line principalDeposits = lst.balanceOf(address(this)); from _splitRewards, we ensure that principalDeposits is only modified by the deposit and withdraw functions, which correctly increment and decrement it based on user actions.

This way, principalDeposits will accurately represent the sum of all user deposits, separate from any rewards earned. It will not be affected by the reward distribution process, maintaining the integrity of the principal tracking.