DPS - FIXED side users receive part of the yield as well which results in losses for the VARIABLE side

[sherlock-admin2]

DPS

High

FIXED side users receive part of the yield as well which results in losses for the VARIABLE side

Summary

Based on the info provided in the README the FIXED side users should receive a premium that is calculated based on the deposits from the VARIABLE side. The VARIABLE side loses their deposits because of this and all of them are distributed as premium for the FIXED side, the VARIABLE side receive yield generated by the stETH. The problem is that the current implementations makes the FIXED side receive part of the yield as well which is wrong in terms of the business logic. This means that FIXED side receive yield and premium while the VARIABLE side receives only yield which disincentivizes the VARIABLE side

Vulnerability Detail

When the vault is started the fixedSidestETHOnStartCapacity is set to the stakingBalance()

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L387

if (
      fixedETHDepositTokenTotalSupply == fixedSideCapacity && variableBearerTokenTotalSupply == variableSideCapacity
    ) {
      startTime = block.timestamp;
      endTime = block.timestamp + duration;
>>    fixedSidestETHOnStartCapacity = stakingBalance();
      fixedIntialTokenTotalSupply = fixedClaimTokenTotalSupply;
      emit VaultStarted(block.timestamp, msg.sender);
    }

This is wrong because the stakingBalance() returns the stEth from Lido. This means that the FIXED side are receiving the whole yield that was generated while the vault was still collecting funds in order to start. We can't calculate how much time it will take for the vault to start because this depends on the users. The protocol always converts the ETH to stEth when someone deposits meaning that we will start generating yield right from the deposit from the first FIXED side participant. Later this fixedSidestETHOnStartCapacity is used when the vault ends and the FIXED side has to get their initial collateral back.

This happens in finalizeVaultEndedWithdrawals() here:

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L677

 function finalizeVaultEndedWithdrawals(uint256 side) external {
    require(side == FIXED || side == VARIABLE, "IS");
    //@note this is updated in vaultEndedWithdraw()
    if (vaultEndedWithdrawalsFinalized) {
      return vaultEndedWithdraw(side);
    }
    require(vaultEndedWithdrawalRequestIds.length != 0 && !vaultEndedWithdrawalsFinalized, "WNR");

    vaultEndedWithdrawalsFinalized = true;

    // claim any ongoing fixed withdrawals too
    claimOngoingFixedWithdrawals();   
    uint256 amountWithdrawn = claimWithdrawals(msg.sender, vaultEndedWithdrawalRequestIds);
>>  uint256 fixedETHDeposit = fixedSidestETHOnStartCapacity;

    if (amountWithdrawn > fixedETHDeposit) {

      vaultEndedStakingEarnings = amountWithdrawn - fixedETHDeposit;
>>    vaultEndedFixedDepositsFunds = fixedETHDeposit;
    } else {
      vaultEndedFixedDepositsFunds = amountWithdrawn;
    }

    uint256 protocolFee = applyProtocolFee(vaultEndedStakingEarnings);
    vaultEndedStakingEarnings -= protocolFee;

    emit LidoWithdrawalFinalized(msg.sender, vaultEndedWithdrawalRequestIds, side, true, true);

    return vaultEndedWithdraw(side);
  }

The vaultEndedFixedDepositsFunds is used to track the amount of ETH used to cover the returning of fixed users initial collateral.

This is the formula in the vaultEndedWithdraw() where the user's gets his initial deposited collateral back

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L748

...MORE CODE

function vaultEndedWithdraw(uint256 side) internal {

 if (side == FIXED) {
      require(
        fixedToVaultOngoingWithdrawalRequestIds[msg.sender].requestIds.length == 0 &&
          fixedToVaultNotStartedWithdrawalRequestIds[msg.sender].length == 0,
        "WAR"
      );

      uint256 sendAmount = fixedToPendingWithdrawalAmount[msg.sender];

      // they submitted a withdraw before the vault had ended and the vault ending should have claimed it
      if (sendAmount > 0) {
        delete fixedToPendingWithdrawalAmount[msg.sender];
      } else {
        uint256 bearerBalance = fixedBearerToken[msg.sender];
        //uint256 bearerBalance = fixedBearerToken.balanceOf(msg.sender);
        require(bearerBalance > 0, "NBT");
>>      sendAmount = fixedBearerToken[msg.sender].mulDiv(vaultEndedFixedDepositsFunds, fixedLidoSharesTotalSupply());

        fixedBearerToken[msg.sender] = 0;   
        fixedBearerTokenTotalSupply -= bearerBalance;    
        vaultEndedFixedDepositsFunds -= sendAmount;
      }

      transferWithdrawnFunds(msg.sender, sendAmount);

      emit FixedFundsWithdrawn(sendAmount, msg.sender, true, true);
      return;

 }

...MORE CODE

As we can see we are multiplying it by the vaultEndedFixedDepositsFunds meaning that he will get more than he initially deposited. He gets more because this variable contains part of the yield that was generated when the vault has not started yet.

Impact

Impact is High because VARIABLE users are not getting the whole yield that was generated from Lido

Likelihood is also High because this happens everytime

Code Snippet

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L387

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L677

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L748

Tool used

Manual Review

Recommendation

When starting the vault just set the to the fixedSideCapacity

  if (
      fixedETHDepositTokenTotalSupply == fixedSideCapacity && variableBearerTokenTotalSupply == variableSideCapacity
    ) {
      startTime = block.timestamp;
      endTime = block.timestamp + duration;
-     fixedSidestETHOnStartCapacity = stakingBalance();
+     fixedSidestETHOnStartCapacity = fixedSideCapacity;
      fixedIntialTokenTotalSupply = fixedClaimTokenTotalSupply;
      emit VaultStarted(block.timestamp, msg.sender);
    }