0x73696d616f - 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 #92
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
Summary
In LidoVault::withdraw(), 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 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
Lido slashes, decreasing the steth ETH / share ratio.
User withdraws, taking a loss and decreasing fixedSidestETHOnStartCapacity with the lossy amount.
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.
0x73696d616f
Medium
Withdrawing after a slash event before the vault has ended will decrease
fixedSidestETHOnStartCapacityby less than it should, so following users will withdraw more their initial depositSummary
In LidoVault::withdraw(), 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 decreasesfixedSidestETHOnStartCapacityby 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 indicates:
Root Cause
In
LidoVault.sol:498,fixedSidestETHOnStartCapacityis 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.
Attack Path
fixedSidestETHOnStartCapacitywith the lossy amount.fixedSidestETHOnStartCapacitywill 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 takes90 ETH * 50 / 100 == 45 ETHinstead due to the loss.fixedSidestETHOnStartCapacityis decreased by45 ETH, the withdrawn amount, so it becomes55 ETH. Now, when LIDO recovers from the slashing, the contract will hold more steth thanfixedSidestETHOnStartCapacity, more specifically the remaining 45 ETH in the contract that were not withdrawn yet are worth 50 ETH now. So user B getsfixedSidestETHOnStartCapacity * 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
fixedSidestETHOnStartCapacityshould be always reduced byfixedETHDeposits.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.