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

9 stars 5 forks source link

0xloophole - Inconsistent Handling of Fixed Side Withdrawals in Ended Vaults #133

Open sherlock-admin3 opened 2 months ago

sherlock-admin3 commented 2 months ago

0xloophole

Medium

Inconsistent Handling of Fixed Side Withdrawals in Ended Vaults

Details

The Lido Vault contract allows users to deposit ETH and participate in Lido staking with both fixed and variable return options. Fixed-side withdrawals are handled differently when the vault ends, especially in loss scenarios where the final ETH balance is lower than the initial fixed-side deposits.

The vaultEndedWithdraw function calculates the amount (sendAmount) to be returned to fixed-side depositors based on vaultEndedFixedDepositsFunds. This value is supposed to reflect the portion of the final balance allocated to fixed users.

However, when the vault ends with losses (vaultEndingETHBalance < fixedSidestETHOnStartCapacity), vaultEndedFixedDepositsFunds is set to vaultEndingETHBalance. This can result in:

Code Snippets

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

if (amountWithdrawn > fixedETHDeposit) {
    vaultEndedStakingEarnings = amountWithdrawn - fixedETHDeposit;
    vaultEndedFixedDepositsFunds = fixedETHDeposit;
} else {
    vaultEndedFixedDepositsFunds = amountWithdrawn;
}

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

sendAmount = fixedBearerToken[msg.sender].mulDiv(
    vaultEndedFixedDepositsFunds,
    fixedLidoSharesTotalSupply()
);

Impact

This inconsistency can lead to inaccurate distribution of remaining funds to fixed-side depositors, potentially overestimating their share in loss scenarios and causing discrepancies or even losses.

Scenario

  1. A vault starts with fixedSidestETHOnStartCapacity of 1 ETH.
  2. The vault ends with vaultEndingETHBalance of 0.8 ETH (a loss).
  3. A fixed-side depositor tries to withdraw.
  4. The code calculates their share based on vaultEndedFixedDepositsFunds (0.8 ETH), potentially overestimating their entitlement and not accounting for the loss.

Fix

Add a check in vaultEndedWithdraw to ensure the calculated sendAmount doesn't exceed the proportional share of the actual vaultEndingETHBalance:

uint256 maxSendAmount = fixedBearerToken[msg.sender].mulDiv(
    vaultEndingETHBalance, 
    fixedLidoSharesTotalSupply()
);
sendAmount = Math.min(sendAmount, maxSendAmount);