Liquid Staking

Summary

In StakingPool::_updateStrategyRewards():L522 function, the total amount staked is updated with the totalRewards variable (which represents the deposit changes across all the strategies). In instances, where totalRewards is negative, the totalStaked state variable is decremented to show an overall loss of staked tokens in the staking pool. In this scenario of loss of staked tokens, the current implementation of the system still mint shares to the strategy fee recipients due to lack of check for a positive net deposit change across the strategies prior to minting the shares, which seems to dilute the shares of the stakers already in the system.

Vulnerability Details

• Before updating the total staked tokens in the system with the totalRewards, it first calculates the fee amounts to be sent to the strategies contracts. That is, the fee for the strategy receivers has already been calculated prior to updating the total staked tokens with the total Rewards variable. #See Below

  https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L540-L547

  if (strategyReceivers.length != 0) {

  receivers[i] = strategyReceivers;

  feeAmounts[i] = strategyFeeAmounts;

  totalFeeCount += receivers[i].length;

  for (uint256 j = 0; j < strategyReceivers.length; ++j) {

  totalFeeAmounts += strategyFeeAmounts[j];

  }

  }

  • After updating the total Staked with the net deposit change across all strategies (total Rewards), it goes on to calculate the list of fees on rewards for the Fees array if there was a positive net change across all the strategies. This means that for a negative net deposits change across strategies, the below if code block will not execute. #See below:

    https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L556-L567

    //@audit-info This if block below will not execute for a negative totalRewards value.

    if (totalRewards > 0) {

    receivers[receivers.length - 1] = new address[]();

    feeAmounts[feeAmounts.length - 1] = new uint256[]();

    totalFeeCount += fees.length;

    ​

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

    receivers[receivers.length - 1][i] = fees[i].receiver;

    feeAmounts[feeAmounts.length - 1][i] =

    (uint256(totalRewards) * fees[i].basisPoints) /

    10000;

    totalFeeAmounts += feeAmounts[feeAmounts.length - 1][i];//@audit-issue totalFeeAmounts has already been initialized to non-zero despite this if block not executing because totalRewards was negative

    }

    }

  • However, because prior to updating totalStaked with the net deposit change across strategies (totalRewards), the totalFeeAmounts to be sent to the strategy receivers has already been initialized and is non-zero. And because, there is no check for whether totalRewards was negative or not in the below if block, the strategy receivers gets minted new shares depsite an overall system loss of staked tokens. #See below:

    https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L576-L597

    //@audit-issue totalFeeAmounts has already been calculated to be non-zero even before updating totalStaked with totalRewards

    if (totalFeeAmounts > 0) {

    uint256 sharesToMint = (totalFeeAmounts * totalShares) /

    (totalStaked - totalFeeAmounts);

    _mintShares(address(this), sharesToMint);

    ​

    uint256 feesPaidCount;

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

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

    if (feesPaidCount == totalFeeCount - 1) {

    transferAndCallFrom(

    address(this),

    receivers[i][j],

    balanceOf(address(this)),

    "0x"

    );

    } else {

    transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");

    feesPaidCount++;

    }

    }

    }

    }

  • This minting of new shares to the strategy receivers causes dilution which unfairly penalizes stakers by reducing their proportional ownership of the system while rewarding strategy receivers despite negative overall system loss of staked tokens across the strategies.

Impact

• Dilution of Stakers: The minting of new shares increases the total share supply, reducing the proportional ownership of existing stakers. In a loss scenario, this means stakers hold a smaller piece of a shrinking pie.

• Misaligned Incentives: Strategy receivers are rewarded irrespective of the overall system’s health. This creates a situation where strategy receivers can profit even if their strategies result in losses for the system, fostering risk-taking behavior that may harm the system long-term.

Tools Used

Manual Review

Recommendations

Add an additional check prior to minting the shares to the strategy receivers to this line of code:

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L576