code-423n4 / 2024-04-lavarage-findings

2 stars 2 forks source link

Malicious borrowers will never repay loans with high interest #10

Open c4-bot-3 opened 4 months ago

c4-bot-3 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-lavarage/blob/9e8295b542fb71b2ba9b4693e25619585266d19e/libs/smart-contracts/programs/lavarage/src/processor/liquidate.rs#L27 https://github.com/code-423n4/2024-04-lavarage/blob/9e8295b542fb71b2ba9b4693e25619585266d19e/libs/smart-contracts/programs/lavarage/src/processor/swap.rs#L16

Vulnerability details

Impact

Borrowers have no incentives to repay the loan if the owed interest grows too much, as the liquidation check fails to take it into consideration when calculating the LTV. This will generate bad debt for the lenders.

Proof of Concept

The liquidation call must pass this check to execute:

require!(ctx.accounts.position_account.amount * 1000 / position_size  > 900, FlashFillError::ExpectedCollateralNotEnough );

https://github.com/code-423n4/2024-04-lavarage/blob/9e8295b542fb71b2ba9b4693e25619585266d19e/libs/smart-contracts/programs/lavarage/src/processor/liquidate.rs#L27

The issue is that it fails to consider how much interest is owed by the borrower, it only checks how much the user has borrowed at the start of the loan:

ctx.accounts.position_account.amount = position_size - user_pays;

https://github.com/code-423n4/2024-04-lavarage/blob/9e8295b542fb71b2ba9b4693e25619585266d19e/libs/smart-contracts/programs/lavarage/src/processor/swap.rs#L16

As such, if the borrower accumulates a very high interest to pay, the lender has no way to liquidate this position if the original borrowed amount's LTV (without accrued interest) stays under 90%. These borrowers will never repaid the loan, and this will generate bad debt for lenders.

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding the owed interest to the total amount when performing the liquidation check.

Assessed type

Invalid Validation

c4-judge commented 4 months ago

alcueca marked the issue as primary issue

c4-sponsor commented 4 months ago

piske-alex (sponsor) confirmed

c4-judge commented 4 months ago

alcueca marked the issue as satisfactory

c4-judge commented 4 months ago

alcueca marked the issue as selected for report

c4-judge commented 3 months ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 3 months ago

alcueca changed the severity to 3 (High Risk)

c4-judge commented 3 months ago

alcueca changed the severity to 2 (Med Risk)

alcueca commented 3 months ago

Downgraded to Medium because even if borrowers can effectively steal the borrowed amount, to do so they need to keep an amount of collateral of higher value locked in the protocol.

DadeKuma commented 3 months ago

@alcueca

I strongly disagree as this clearly warrants High severity if we follow the severity categorization:

3 — High: Assets can be stolen/lost/compromised directly (or indirectly if there is a valid attack path that does not have hand-wavy hypotheticals).

There are zero hypotheticals, any borrower can get a loan for an unlimited amount of time (and not pay ANY interest), without consequences.

It's like saying that I go to the bank to get a loan, never pay the interest (without consequences), and they can't liquidate me until the collateral I provided is worthless. The bank is experiencing a loss of funds because it is lending money for free.

alcueca commented 3 months ago

And the attacker is experiencing a larger loss of funds, which makes it a griefing attack, which is a Medium.

alcueca commented 3 months ago

Please be mindful of tone, expressed through format options such as bold. This discussion is to be kept within certain boundaries.

DadeKuma commented 3 months ago

@alcueca

I apologize if my previous message came across as disrespectful; that was not my intention. I used bold text to clarify how and where the funds are lost as I wrote many paragraphs, my only goal was to increase the legibility of my explanation, not being disrespectful to you.

Anyway, consider the following case. The lender lends 1000 SOL with a 10% monthly rate. Let's say that the collateral is worth 1500 SOL.

After 6 months, the lender has lost more funds than the borrower (they should have earned 600 SOL but they have earned 0), and they can't do anything to claim the collateral, they are lending money for free. But they are forced to wait until the collateral is worthless.

Consequences: 1) Lenders miss interest payments potentially forever 2) In the meanwhile, they can't offer new loans, as their funds are already locked in this one 3) The protocol is useless if no one repays their loans as lenders only lose money 4) If accrued interest is higher than collateral, the borrower will never repay, because at this point the liquidation costs less than the repayment

The lender is accruing bad debt, which is a clear loss of funds for the lender. Recouping the collateral is not enough if their funds stay locked for decades.

alcueca commented 3 months ago

When measuring financial losses it is uncommon to consider loss of future income or opportunity cost.

I see this vulnerability as the victim putting X assets into the protocol, and the attacker spending Y assets so that the victim loses theirs, with the value of Y greater than the value of X.

If the victim would have put their assets somewhere expecting 0% income, for example in an escrow contract, and they would get locked in the same fashion, you could still make the reasoning that because the victim has been blocked from withdrawing and earning an income on their assets in perpetuity, the issue is a critical.

The reasoning can be extended to make all griefing attacks of critical severity, which isn't fair to attacks where the attacker obtains an actual profit.

DadeKuma commented 3 months ago

It's not future income as interest is accrued daily. High risk means that funds can be compromised directly (it's worth noting that this isn't a dust amount, and any borrower can do it):

3 — High: Assets can be stolen/lost/compromised directly (or indirectly if there is a valid attack path that does not have hand-wavy hypotheticals).

The actual rules say nothing about griefing, and borrowing/lending is a core feature of this protocol.

There are multiple scenarios:

1) Scenario 1: The borrower locks collateral but they recoup some immediately by taking the borrowed amount. The lender has no access to any funds until liquidation, which can't be enforced as interest is not accrued. For a % cost of the total borrowed amount, the attacker can lock a sizeable capital forever. 2) Scenario 2: There is no attacker, and a borrower has lost access to their wallet. The loan remains unliquidable forever, and the lender loses 100% of the capital as there is no time limit for a loan (like I described in #9 which was duped to this issue).

Picodes commented 3 months ago

Lavarage Appellate Court Decisions

Summary

Issue #10 describes how because loans have no fixed duration and interests are not taken into account into the liquidiation mechanism, lenders cannot count on interests to trigger a liquidation at some point in the future, and there is a point time after which it isn't profitable anymore for the borrower to pay back its loan. It's related to #9 which focuses on the business implications of having infinite duration loans.

Sponsor's input

The sponsor was asked for his input on the original design he had in mind and the value added by #9 and #10. Here is his answer:

"I can confirm that the design I am willing to implement is that loans do not have a fixed term. But I also agree that that could be a risk on the business logic side. However, we are looking into implementing interest payment collection through sales of collateral instead of setting a fixed time for the loans. As mentioned above I agree that #9 is a valid concern in regards of business logic design. I don't think we have discussed it in our documentation."

0xTheC0der’s view

I am viewing this from the following perspectives:

Leaning towards High severity due to the indefinite lockup of collateral assets even without malicious user intent which effectively translates into a loss of the value of the borrowed assets. While the warden deserves to have their finding upgraded to High severity, this is a borderline case and also the contest judge's assessment of Medium severity seems to hold under C4 rules and therefore cannot be labeled a "clear mistake" as the appellate court rules currently suggest in case of a 3/3 agreement on High severity. Consequently, I want my final verdict to be interpreted in such a way that a 2/3 agreement on High severity is reached.

Picodes' view

I think H is more appropriate as well. For sure there will be users leverage trading and losing their borrowed amount, losing their keys, etc, and the main backstop for lenders is that due to interests they will get their collateral back at some point. That's the classic behavior for Aave, Compound, Morpho, etc. Without this they can only rely on price movements which isn't the deal. As a proof I think Aave V2 or deprecated lending markets speaks for themselves where lenders are just waiting for liquidations to be able to withdraw and the amounts at stake are significants

Hickuphh3’s view

I think H severity is more appropriate, not because of the loss of unrealised yield (this would be M), but because of the collateral that the lender is entitled to can be indefinitely locked, as long as LTV is <= 90%.

Should interest accrual be accounted for, the position will eventually be liquidatable at some point, but excluding it means the condition may only be met from asset price changes

Verdict

By a 2/3 consensus, the conclusion from the appellate court is that the ruling should be overruled and the issue should be made of High severity.