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

9 stars 5 forks source link

tobi0x18 - Incorrect earning calculation while vault is in active #146

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

tobi0x18

High

Incorrect earning calculation while vault is in active

Summary

When the variable depositors withdraw their earnings, the protocol calculates the totalEarnings and previousWithdrawnAmount using shares and the stETH balance. However, the calculation is incorrect because it multiplies the current stETH balance per stake by the previous withdrawn stakes. As a result, the variable depositors receive incorrect earnings, and the fixed depositors may receive a smaller amount.

Root Cause

There is an incorrect calculation of the totalEarnings and previousWithdrawnAmount parameters in the calculateVariableWithdrawState function within the LidoVault.withdraw function.

        (uint256 currentState, uint256 ethAmountOwed) = calculateVariableWithdrawState(
            (lidoStETHBalance.mulDiv(currentStakes + withdrawnStakingEarningsInStakes, currentStakes) - fixedETHDeposits),
            variableToWithdrawnStakingEarningsInShares[msg.sender].mulDiv(lidoStETHBalance, currentStakes)
        );

If the stETH balance per stake differs from the previous balance per stake, the totalEarnings is calculated incorrectly because it uses withdrawnStakingEarningsInStakes. Additionally, previousWithdrawnAmount is calculated incorrectly because the previous withdrawn shares are multiplied by the current balance per stake.

totalEarnings = lidoStETHBalance.mulDiv(currentStakes + withdrawnStakingEarningsInStakes, currentStakes) - fixedETHDeposits
previousWithdrawnAmount = variableToWithdrawnStakingEarningsInShares[msg.sender].mulDiv(lidoStETHBalance, currentStakes)

There is also an incorrect calculation of the totalEarnings and previousWithdrawnAmount parameters in the calculateVariableWithdrawState function within the LidoVault.vaultEndedWithdraw function.

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

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

Internal pre-conditions

  1. There is a vault and Alice and Bob are variable side depositors and they deposits same ETH.
  2. The vault is started with the following initial values.
    • fixedSideCapacity: 1000 wei (use wei instead of ETH for the simplicity)
    • variableSideCapacity: 30 wei
    • fixedSidestETHOnStartCapacity: 1000 wei
    • fixedClaimTokenTotalSupply: 1000
    • protocolFeeBps: 0 (for the simplicity)
  3. At time start < t1 < end, lido gets 100 wei profit.
  4. At time start < t2 < end, lido also gets 100 wei profit.

External pre-conditions

No response

Attack Path

Let's consider the following scenario:

currentStakes = 1000
fixedETHDeposits = 1000
totalEarnings = lidoStETHBalance.mulDiv(currentStakes + withdrawnStakingEarningsInStakes, currentStakes) - fixedETHDeposits = 1100 - 1000 = 100.
previousWithdrawnAmount = 0
totalOwed = bearerBalance.mulDiv(totalEarnings, variableSideCapacity) = 100 / 2 = 50
ethAmountOwed = 50 - 0 = 50.
stakesAmountOwed = lido.getSharesByPooledEth(ethAmountOwed) = 50 * 1000 / 1100 = 45
withdrawnStakingEarningsInStakes = 45
variableToWithdrawnStakingEarnings[`Alice`] = 50
variableToWithdrawnStakingEarningsInShares[`Alice`] = 45
lidoStETHBalance = 1050
currentStakes = 1000 - 45 = 955
totalEarnings = lidoStETHBalance.mulDiv(currentStakes + withdrawnStakingEarningsInStakes, currentStakes) - fixedETHDeposits = 1250 * 1000 / 955 - 1000 = 308
previousWithdrawnAmount = 0
totalOwed = bearerBalance.mulDiv(totalEarnings, variableSideCapacity) = 308 / 2 = 154
ethAmountOwed = 154 - 0 = 154
stakesAmountOwed = lido.getSharesByPooledEth(ethAmountOwed) = 154 * 955 / 1250 = 117
withdrawnStakingEarningsInStakes = 45 + 117 = 162
variableToWithdrawnStakingEarnings[`Bob`] = 154
variableToWithdrawnStakingEarningsInShares[`Bob`] = 117
lidoStETHBalance = 1250 - 154 = 1096

Total earnings is 300 and Alice and Bob should receive same earnings because they deposits the same ethers. However, Bob receives 154 instead of 150 and this is unfair for Alice.

Impact

The variable user may receive unfair earnings.

PoC

No response

Mitigation

The variable depositors should not be allowed to withdraw earnings before the vault ended.