Liquid Staking

Github

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/linkStaking/OperatorVCS.sol#L395-L397

Summary

When the admin sets a new _rewardsReceiver for a vault, if the previous rewards receiver has unclaimed rewards, those unclaimed rewards will go to the new rewards receiver when updateDeposits() is called. This results in the new receiver getting all previously accrued rewards, even those meant for the old receiver.

Impact

The original rewards receiver may lose their rightful rewards if a new receiver is set before the old receiver claims them. This could lead to disputes over unclaimed rewards and result in loss of rewards for the original intended recipient.

Cause

The setRewardsReceiver() function allows for setting a new rewards receiver without requiring the old receiver to claim their accrued rewards first. When updateDeposits() is called, any unclaimed rewards are transferred to the new receiver, bypassing the old receiver's rights to claim those rewards.

Likelihood

This issue is highly likely to occur in scenarios where the admin changes the rewards receiver without ensuring the old receiver has claimed their rewards.

Proof of Concept

1. The admin calls setRewardsReceiver() to update the rewards receiver for a vault.

2. The old rewards receiver has unclaimed rewards.

3. The new rewards receiver is set without the old rewards being claimed.

4. When updateDeposits() is called, the unclaimed rewards are transferred to the new rewards receiver, leaving the old receiver without their earned rewards.

Recommendations

Before setting a new rewards receiver, ensure that the current rewards receiver claims all accrued rewards. This can be enforced by requiring a rewards claim before allowing the setRewardsReceiver() function to execute.