An incorrect income distribution will lead to fund losses during slashing

Summary

A variable user can withdraw their income before the Saffron Lido Vault concludes. However, total income may decrease due to slashing, allowing variable users to withdraw despite this reduction. Consequently, some users who delay their withdrawals may find that insufficient ethers remain in the vault to cover their funds.

Root Cause

In LidoVault.sol:775, the calculation of totalEarnings is incorrect, particularly during slashing events, which could lead to potential fund losses for users.

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

775:  uint256 totalEarnings = vaultEndingETHBalance.mulDiv(withdrawnStakingEarningsInStakes,vaultEndingStakesAmount) - totalProtocolFee  + vaultEndedStakingEarnings;

777:  if (totalEarnings > 0) {
        (uint256 currentState, uint256 stakingEarningsShare) = calculateVariableWithdrawState(
          totalEarnings,
          variableToWithdrawnStakingEarningsInShares[msg.sender].mulDiv(vaultEndingETHBalance, vaultEndingStakesAmount)
        );
        stakingShareAmount = stakingEarningsShare;
        variableToWithdrawnStakingEarningsInShares[msg.sender] = currentState.mulDiv(vaultEndingStakesAmount,vaultEndingETHBalance);
        variableToWithdrawnStakingEarnings[msg.sender] = currentState;
      }

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

    uint256 amountWithdrawn = claimWithdrawals(msg.sender, vaultEndedWithdrawalRequestIds);
    uint256 fixedETHDeposit = fixedSidestETHOnStartCapacity;
    if (amountWithdrawn > fixedETHDeposit) {
      vaultEndedStakingEarnings = amountWithdrawn - fixedETHDeposit;
      vaultEndedFixedDepositsFunds = fixedETHDeposit;
    } else {
      vaultEndedFixedDepositsFunds = amountWithdrawn;
    }

Internal pre-conditions

There is a vault with the following parameters. fixedSideCapacity : 100 ETH variableSideCapacity : 3 ETH

External pre-conditions

Slashing occurs in the Lido Protocol.

Attack Path

Alice deposits 100 ETH.(a fixed user)
Bob deposits 2 ETH and Charlie deposits 1 ETH.(variable users)
The stETH balance of vault increases to 103 ETH.
Bob withdraw his income (103 - 100) * 2 / 3 = 2 ETH. (For simplicity, we assume no protocol fee)
The vault ends when the stETH balance of vault increases to from 101 ETH to 99 ETH.
In LidoVault.sol:775, vaultEndedStakingEarnings is 0, because 99 < 101 (See LidoVault.sol:L673-L680). totalProtocolFee = 0. vaultEndingETHBalance.mulDiv(withdrawnStakingEarningsInStakes,vaultEndingStakesAmount) is about 2 ETH. So, totalEarnings is about 2ETH.
Charlie can withdraw income of about 1ETH.
Alice should withdraw 99ETH.

As a result of the above scenario, at least one of Alice and Charlie cannot withdraw their fund because there is no enough ether.

Impact

Some fixed user may not return back his principal.
Some variable user may not receive his income.

PoC

No response

Mitigation

Variable users should not be allowed to withdraw their income before the end. Or income distribution mechanism should be improved.

sherlock-audit / 2024-08-saffron-finance-judging

tobi0x18 - An incorrect income distribution will lead to fund losses during slashing #136