Skip to content
Request an audit

‹ All findings

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

MediumSaffron Lido Vaults·Sherlock · Staking · 16th September 2024CodebaseM-2

Source

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

  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.