<!-- canonical: https://0xsimao.com/findings/saffron-lido-vaults-slash-eth-withdraw-deposit -->

# Withdrawing after a slash event before the vault has ended will decrease `fixedSidestETHOnStartCapacity` by less than it should, so following users will withdraw more their initial deposit

Medium · Sherlock · Staking · 16th September 2024

Finding M-2 of the Saffron Lido Vaults competition.

- Protocol: https://audits.sherlock.xyz/contests/509
- Codebase: https://github.com/0xsimao/2024-08-saffron-finance/tree/38dd9c8436db341c331f1b14545770c1766fc0ee
- Source: https://github.com/sherlock-audit/2024-08-saffron-finance-judging/issues/92

---

### Summary

In [LidoVault::withdraw()](https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L423), when the vault has started but not ended, it limits the value to withdraw if a slashing event occured and withdraws `lidoStETHBalance.mulDiv(fixedBearerToken[msg.sender], fixedLidoSharesTotalSupply());`. However, it also decreases `fixedSidestETHOnStartCapacity` by this same amount, which means that next users that withdraw will get more than their initial deposit in case the Lido ratio comes back up (likely during the vault's duration).

It's clear from the code users should get exactly their initial amount of funds or less, never more, as the [comment](https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L486) indicates:
> since the vault has started only withdraw their initial deposit equivalent in stETH  at the start of the vault- unless we are in a loss

### Root Cause

In `LidoVault.sol:498`, `fixedSidestETHOnStartCapacity` is decreased by a lower amount than it should.

### Internal pre-conditions

None.

### External pre-conditions

Lido slash, which is in scope as per the readme.
> The Lido Liquid Staking protocol can experience slashing incidents (such as this https://blog.lido.fi/post-mortem-launchnodes-slashing-incident/). These incidents will decrease income from deposits to the Lido Liquid Staking protocol and could decrease the stETH balance. The contract must be operational after it

### Attack Path

1. Lido slashes, decreasing the steth ETH / share ratio.
2. User withdraws, taking a loss and decreasing `fixedSidestETHOnStartCapacity` with the lossy amount.
3. Next user withdrawing will withdraw more because `fixedSidestETHOnStartCapacity` will be bigger than it should.

### Impact

Fixed deposit users benefit from the slashing event at the expense of variable users who will take the loss.

### PoC

Assume that there 100 ETH and 100 shares.
A slashing event occurs and drops the ETH to 90 and shares remain 100.
There are 2 fixed depositors, with 50% of the deposits each.
User A withdraws, and should take `100 ETH * 50 / 100 == 50 ETH`, but takes `90 ETH * 50 / 100 == 45 ETH` instead due to the loss.
`fixedSidestETHOnStartCapacity` is decreased by `45 ETH`, the withdrawn amount, so it becomes `55 ETH`.
Now, when LIDO recovers from the slashing, the contract will hold more steth than `fixedSidestETHOnStartCapacity`, more specifically the remaining 45 ETH in the contract that were not withdrawn yet are worth 50 ETH now. So user B gets
`fixedSidestETHOnStartCapacity * 50 / 50 == 55`. 

As the fixed deposit user initially deposited 50, but claimed 55 now, it is getting much more than it should at the expense of the variable users who will take the loss.

### Mitigation

The `fixedSidestETHOnStartCapacity` should be always reduced by `fixedETHDeposits.mulDiv(fixedBearerToken[msg.sender], fixedLidoSharesTotalSupply());`, such that users get their equivalent ETH from their initial deposit back and the variable users don't take losses.

---

Related findings:

- [Donation attack possible, although unlikely, could make an initial deposit](https://0xsimao.com/findings/aegis-donation-although-unlikely-deposit): Aegis Staked YUSD
- [Incorrect initial deposit calculation may cause cancelProgram to revert](https://0xsimao.com/findings/superfluid-locker-system-ii-initial-deposit-program-revert): Superfluid Locker Pumponomics
- [`ManagedLeveragedVault::decreaseLeverage()` will not work when it goes below the minimum debt](https://0xsimao.com/findings/beraborrow-i-decrease-goes-below-debt): Beraborrow Managed Dens
- [Users redeeming early will withdraw `Ra` without decreasing the amount locked, which will lead to stolen funds when withdrawing after expiry](https://0xsimao.com/findings/cork-protocol-redeeming-withdraw-locked-stolen): Cork Protocol
