code-423n4 / 2023-12-initcapital-findings

3 stars 3 forks source link

bad debt is not socialized #15

Closed c4-bot-6 closed 8 months ago

c4-bot-6 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-initcapital/blob/main/contracts/core/InitCore.sol#L288

Vulnerability details

Proof of Concept

In case if borrower's position is unhealthy, then he can be liquidated. Liquidator can provide amount of shares in _poolToRepay that he will cover and expects to get back _poolOut shares. It is possible that position created a bad debt. This means that amount that position has borrowed is bigger than supplied collateral.

In this case only part of debt of the positon will be repaid to lenders by liquidator. Let's check what that means for the pool.

When burn is called, then _toAmt is called to calculate amount that user can receive for his shares. It uses cash + totalDebt and totalSupply to calculate it. totalDebt is all borrowed amount that is not repaid yet. So contract expects that all borrowed amount will be repaid eventually.

So in case if bad debt occurs it means that part of borrowed amount will not be received back by LendingPool contract, however contract still calculates assets using outdated data.

As result this bad debt will create contract insolvency, which means that contract will not have enough balance to pay last burners.

Impact

Bad debt is not distributed among all lenders

Tools Used

Recommended Mitigation Steps

You should track when position is closed with bad debt and then notify LendingPool contract that borrower will not be able to return part of funds. This should decrease totalDebt amount, which will decrease share price and thus distribute that debt among all lenders.

Assessed type

Error

c4-judge commented 8 months ago

hansfriese marked the issue as duplicate of #9

fez-init commented 8 months ago

@hansfriese This issue is different from #9 .

hansfriese commented 8 months ago

@fez-init Would you explain in detail what the differences are? From my understanding, both issues are saying totalDebt should be updated properly by distributing a debt among all lenders when a position is closed with a bad debt.

fez-init commented 8 months ago

@hansfriese This issue talks about a recommendation where bad debt should be socialized to all lenders.

9 talks about the case of tx revert when bad debt occurs. When bad debt occurs, what will likely happen is:

1) a liquidator can come in and repay partial debt and take ALL the collaterals (since there are liquidation premiums). So we are left with 0 collateral and >0 debt. 2) The idea is that we would like to be able to liquidate this debt as well. However, in the liquidate function, it will try to removeCollateral to the liquidator, and there's a check that requires share > 0 so the secondary liquidation will always undesirably revert.

c4-sponsor commented 8 months ago

fez-init marked the issue as disagree with severity

fez-init commented 8 months ago

This issue should be marked as part of low or QA since it is more of a recommendation to the design choice.

c4-sponsor commented 8 months ago

fez-init (sponsor) acknowledged

hansfriese commented 8 months ago

Downgrade to QA as it doesn't show the main impact.

c4-judge commented 8 months ago

hansfriese marked the issue as not a duplicate

c4-judge commented 8 months ago

hansfriese changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

hansfriese marked the issue as grade-c

rvierdiiev commented 8 months ago

hello @hansfriese I understand that #9 moves 1 step further and discovers that liquidation is not possible when position was liquidated and bad debt have occured. However i do not agree that this report doesn't show the main impact. Because for me the main impact is not disability to repay bad debt loan by anyone else, but the main impact is system insolvency.

While i agree that protocol is able(and likely will) to repay some bad debt, i didn't see any solution from protocol side to do that. I mean that in case of big amount of bad debt, it can be not enough just to use protocol's fee as protocol fee is not guaranteed to stay in the contracts and can be withdrawn at any time.

Insolvency was always bigger than qa, that's why i believe this is medium severity issue.

hansfriese commented 8 months ago

@rvierdiyev I totally understand your point.

  1. I agree the main impact is the system insolvency and it's worth keeping this report as Medium because there is no specific solution to resolve.
  2. 9 shows a more severe impact by clarifying that it's impossible to liquidate positions with 0 collaterals.

    For the above reasons, I think it's appropriate to mark this one as a dup of #9 with 50% partial credit.

c4-judge commented 8 months ago

hansfriese removed the grade

c4-judge commented 8 months ago

This previously downgraded issue has been upgraded by hansfriese

c4-judge commented 8 months ago

This previously downgraded issue has been upgraded by hansfriese

c4-judge commented 8 months ago

hansfriese marked the issue as duplicate of #9

c4-judge commented 8 months ago

hansfriese marked the issue as partial-50

c4-judge commented 8 months ago

hansfriese marked the issue as satisfactory

c4-judge commented 8 months ago

hansfriese marked the issue as partial-50