sherlock-audit / 2023-01-ajna-judging

1 stars 0 forks source link

james_wu - After auction settlement, remaining loan is not re-inserted to heap #172

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

james_wu

high

After auction settlement, remaining loan is not re-inserted to heap

Summary

Any remaining loans after the auction settlement will not be reinserted into the heap. Therefore, the number of loans is wrong (decrease by 1). And Loans.remove() is called again to remove other loan.

Vulnerability Detail

As a result of settle, there's case of borrower's debt and collateral being remained. Auction is removed, but loan isn't re-inserted into the loans heap. It occures wrong noOfLoans, while totalDebt and totalCollateral are correct.

First, poolMinDebtAmount_ are mis calculated. Second, let's assume that after settle, there is Debt and Collateral remaining for borrower. When someone call _kick(), it mis-calculate the momp due to wrong noOfLoans, and furthermore, it calls Loans.remove() and remove wrong loan completely.

Impact

_wrong momp calculation in kick is wrong bond size and loss of funds for kicker remove wrong loan, so loss of funds for depositors

Code Snippet

https://github.com/sherlock-audit/2023-01-ajna/blob/main/contracts/src/libraries/external/Auctions.sol#L749-L840

// calculate auction params
uint256 noOfLoans = Loans.noOfLoans(loans_) + auctions_.noOfAuctions;

uint256 momp = _priceAt(
    Deposits.findIndexOfSum(
        deposits_,
        Maths.wdiv(poolState_.debt, noOfLoans * 1e18)
    )
);
// remove kicked loan from heap
Loans.remove(loans_, borrowerAddress_, loans_.indices[borrowerAddress_]);

Tool used

Manual Review

Recommendation

You should re-insert the loan right after auction is settled.

grandizzy commented 1 year ago

after auction is settled Loans.update is called which readd Loan in heap if case. This happens in BorrowerAction.repayDebt at line 338 (debt repaid), BorrowerAction.drawDebtat line 199 (more collateral pledged) and Auctions._takeLoan at line 1050 (take / bucketTake settles auction). It is not performed when settlePoolDebt called as the aim of this is to recover the entire debt from auction and a loan with 0 debt is not added in heap

hrishibhat commented 1 year ago

Closing based on Sponsor comments