Yield of LiquidityReserve can be stolen

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L126
https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L176
https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L206

Vulnerability details

Impact

Using sandwich attacks and JIT (Just-in-time liquidity), the yield of LiquidityReserve could be extracted for liquidity providers.

Proof of Concept

The yield of LiquidityReserve is distributed when a user calls instantUnstakeReserve() in Staking. Then, in instantUnstake, totalLockedValue increases with the fee paid by the user withdrawing. The fee is shared between all liquidity providers as they all see the value of their shares increase.

Therefore, an attacker could do the following sandwich attack when spotting a call to instantUnstakeReserve().

• In a first tx before the user call, borrow a lot of stakingToken and addLiquidity
• The user call to instantUnstakeReserve()leading to a fee of sayx`
• In a second tx after the user call, removeLiquidity and repay the loan, taking a large proportion of the user fee

The problem here is that you can instantly add and remove liquidity without penalty, and that the yield is instantly distributed.

Recommended Mitigation Steps

To mitigate this, you can

• store the earned fees and distribute them across multiple blocks to make sure the attack wouldn’t be worth it
• add a small fee when removing liquidity, which would make the attack unprofitable
• prevent users from withdrawing before X blocks or add a locking mechanism

[0xean]

This is not unique to the protocol and is a vulnerability in almost all of the LP designs that are prevalent today. There is no loss of user funds here either.

Would downgrade too Low or QA.

[Picodes]

In standard cases of JIT, for example in a DEX, the attacker takes a risk as the liquidity he adds is used during the swap, and this liquidity is useful for the protocol as leads to a better price for the user, which is not the case here

[0xean]

@Picodes - that is fair but the liquidity is still useful and I still don't see how this qualifies as high severity. Eventually it would mean that the liquidity reserve would need less liquidity parked in it if JITers always where hitting it.

[Picodes]

To me it's high because: (correct me if I am missing things)

• JIT is not useful here at all for the protocol, the liquidity they bring is not useful as does not get locked. It's totally risk free, and as you said it’s a commun attack so it’s likely that someone uses it
• It leads to a loss of LP funds:
  Assume there is 100k unlocked in the pool, and someone instantUnstake 100k, it’ll lock all the LP liquidity. But if someone JITs this, the fees will go to the attacker and not the LP which provided the service by accepting to have its liquidity locked.
• From a protocol point of view, LPing becomes unattractive as all the fees are stolen, breaking the product design

[moose-code]

Agree going to leave this as high. Any whale that does a large unstake will be susceptible to having more of the fee's eroded to a predatory sandwich attack which provides no value to the system.

[Picodes]

@JeeberC4 what is the duplicate issue ?

[Picodes]

@moose-code, @JasoonS, @DenhamPreen this is flagged as a duplicate of H-06, is it a mistake ?

[JasoonS]

Thanks, seems it was made a duplicate by mistake.