Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sherlock-admin4 opened this issue Sep 21, 2024 · 0 comments
Open

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Sep 21, 2024

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:

  • At time t1, the lidoStETHBalance is 1100 wei after gaining a profit of 100 wei, and Alice calls the withdraw function.
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
  • At time t2, the lidoStETHBalance is 1250 wei after gaining a profit of 200 wei, and Bob calls the withdraw function.
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.

@sherlock-admin4 sherlock-admin4 changed the title Festive Marmalade Unicorn - Incorrect earning calculation while vault is in active tobi0x18 - Incorrect earning calculation while vault is in active Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant