Liquid Staking

Summary

In the StakingPool::_updateStrategyRewards function, rewards and fees are distributed based on balance changes in strategies since the last update. However, there is an oversight in how strategy::updateDeposits and StakingPool::_updateStrategyRewards are integrated. This causes incorrect calculation of the shares to be minted for fee receivers, as the operatorRewards are either incorrectly excluded or partially included in the total fee amounts. Additionally, the operator rewards accounting is flawed, allowing operators to withdraw more value than intended.

Vulnerability Details

The StakingPool::_updateStrategyRewards function calls strategy::updateDeposits, which returns the receivers and amounts arrays. However, the OperatorVCS::updateDeposits implementation has two different cases that populate the amounts array:

1. When operatorRewards != 0:

   if (operatorRewards != 0) {

   receivers = new address[](1 + (depositChange > 0 ? fees.length : 0));

   amounts = new uint256[](receivers.length);

   receivers[0] = address(this);

   amounts[0] = operatorRewards;

   unclaimedOperatorRewards += operatorRewards;

   }

2. When depositChange > 0:

   if (depositChange > 0) {

   newTotalDeposits += uint256(depositChange);

   if (receivers.length == 0) {

   receivers = new address[](fees.length);

   amounts = new uint256[](receivers.length);

   for (uint256 i = 0; i < receivers.length; ++i) {

   receivers[i] = fees[i].receiver;

   amounts[i] = (uint256(depositChange) * fees[i].basisPoints) / 10000;

   }

   }

   }

In some cases, the receivers and amounts arrays set earlier in the operatorRewards block can be overwritten by the depositChange logic, or both operatorRewards and depositChange may be combined. This inconsistency results in incorrect summation of rewards in StakingPool::_updateStrategyRewards:

The totalFeeAmounts value is later used to calculate the shares to mint (sharesToMint) and distribute to fee receivers. However, the shares minted can be incorrect since parts of the operatorRewards are not properly factored in, resulting in inaccurate fee distribution.

Issue 2:
Additionally, the variables tracking operator rewards, such as unclaimedOperatorRewards and unclaimedRewards, are not updated, allowing operators to withdraw more rewards than intended, when they later claim rewards.

Impact

• Issue 1: Incorrect Shares Minted for Fee Receivers
  The amount of shares minted for fee receivers is incorrect due to inconsistent accounting of operatorRewards and depositChange. This leads to an unfair distribution of rewards.

• Issue 2: Undercounting of Operator Rewards
  Since the variables tracking operator rewards (unclaimedOperatorRewards in OperatorVCS and unclaimedRewards in OperatorVault) are not updated when rewards are distributed, operators can withdraw more than their fair share of rewards, resulting in excess claims and potential financial loss for the system.

Tools Used

Manual

Recommendations

1. Ensure that both operatorRewards and depositChange are accurately included in the total fee amounts calculated in StakingPool::_updateStrategyRewards.

2. Update the unclaimedOperatorRewards and unclaimedRewards variables in OperatorVCS and OperatorVault when operator rewards are distributed to prevent double-counting or excess withdrawal of rewards by operators.