code-423n4 / 2023-06-lybra-findings

8 stars 7 forks source link

Fixed reward percentage for liquidators in the eUSD vault may cause a liquidation crisis #72

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/5d70170f2c68dbd3f7b8c0c8fd6b0b2218784ea6/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L154-L176

Vulnerability details

Impact

To not lose generality, the same issue is present in the LybraPEUSDVaultBase contract.

Liquidations are essential for a lending protocol to maintain the over-collateralization of the protocol. Hence, when a liquidation happens, it should increment the collateral ratio of the liquidated position (make it healthier).

The LybraEUSDVaultBase contract has a function named liquidation, which is used to liquidate users whose collateral ratio is below the bad collateral ratio, which for the eUSD Vault is 150%. This function incentives liquidators with a fixed reward of 10% of the collateral being liquidated. However, the issue with the fixed compensation is that it will cause a position to get unhealthier during a liquidation when the collateral ratio is 110% or smaller.

Proof of Concept

Take the following example:

The data above will give us a collateral ratio for the position of: 107.9%. The liquidator liquidates the max amount possible, which is 50% of the collateral, one ether, and takes 10% extra for its services; the final collateral ratio will be:

((2 - 1.1) * 1500) / (2779 - 1500) = 1.055

The position got unhealthier after the liquidation, from a collateral ratio of 107.9% to 105%. The process can be repeated until it is no longer profitable for the liquidator leading the protocol to accumulate bad debt.

Justification

I landed medium on this finding for the following reasons:

Tools Used

Manual Review

Recommended Mitigation Steps

When a position has a collateral ratio below 110%, the reward percentage should be adjusted accordingly instead of a fixed reward of 10%.

Assessed type

Math

c4-pre-sort commented 1 year ago

JeffCX marked the issue as primary issue

c4-sponsor commented 1 year ago

LybraFinance marked the issue as disagree with severity

LybraFinance commented 1 year ago

Liquidation due to a collateral ratio below 110% results in a further decrease in the collateral ratio. When the collateral ratio falls below 100%, the liquidation outcome remains the same. In practice, liquidation occurs when the collateral ratio reaches 150%, making it unlikely to have an excessively low collateral ratio.

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Invalid

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

0xRobocop commented 1 year ago

I think this issue was misjudged:

The issue describes how an overcollateralized position between 101% and 109% (inclusive) gets unhealthier with each liquidation. From my knowledge, there is no CDP protocol that allows this behavior since instead of helping the protocol increment the total collateral ratio, it accelerates to go to lower levels.

I think this is a clear med issue because it has the requirement of a position to reach a CR of 109% which may not happen, but still it cannot be guaranteed that it will never happen and the protocol should handle these cases.

Med issue definition:

Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

0xean commented 1 year ago

Thanks @0xRobocop for making your case, I think its on the cusp between being a design decision (which I agree is a sub optimal design choice) and a M severity issue.

Liquidation's cascading into to more liquidations is never a good outcome and I think you are correct that this does lead to that. Lets get some sponsor comment and for the moment, I will upgrade to M

c4-judge commented 1 year ago

This previously downgraded issue has been upgraded by 0xean

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

0xean commented 1 year ago

@LybraFinance

LybraFinance commented 1 year ago

Although we have been using this design since V1, we have to admit, liquidation's cascading into to more liquidations is never a good outcome. Therefore, we have decided to modify this logic. Thank you for your valuable suggestions!

c4-sponsor commented 1 year ago

LybraFinance marked the issue as sponsor confirmed

c4-judge commented 1 year ago

0xean marked the issue as selected for report