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

0 stars 0 forks source link

Function `settleWithBuyout()` does not call `LoanManager.loanLiquidation()` during a buyout #49

Open c4-bot-3 opened 7 months ago

c4-bot-3 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/AuctionWithBuyoutLoanLiquidator.sol#L83

Vulnerability details

Impact

Lenders in the Gondi protocol could be EOA and Gondi Pool. Gondi Pool, an ERC4626, allows anyone to deposit funds and earn yield from lending on Gondi. Gondi Pool implemented the LoanManager interfaces, which include the validateOffer(), loanRepayment(), and loanLiquidation() functions. The functions loanRepayment() and loanLiquidation() are called when a borrower repays the loan or the loan is liquidated, i.e., when the Pool receives funds back from MultiSourceLoan. Both functions is used to update the queue accounting and the outstanding values of the Pool.

ERC20 asset = ERC20(_auction.asset); 
uint256 totalOwed;
// @audit Repay lender but not call LoanManager.loanLiquidation()
for (uint256 i; i < _loan.tranche.length;) {
    if (i != largestTrancheIdx) { 
        IMultiSourceLoan.Tranche calldata thisTranche = _loan.tranche[i];
        uint256 owed = thisTranche.principalAmount + thisTranche.accruedInterest
            + thisTranche.principalAmount.getInterest(thisTranche.aprBps, block.timestamp - thisTranche.startTime);
        totalOwed += owed; 
        asset.safeTransferFrom(msg.sender, thisTranche.lender, owed);
    }
    unchecked {
        ++i;
    }
}
IMultiSourceLoan(_auction.loanAddress).loanLiquidated(_auction.loanId, _loan);

In the settleWithBuyout() function, the main lender buys out the loan by repaying all other lenders directly. However, loanLiquidation() is not called, leading to incorrect accounting in the Pool.

Proof of Concept

The loanLiquidation() function handles accounting in the pool. https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L449-L463

function loanLiquidation(
    ...
) external override onlyAcceptedCallers {
    uint256 netApr = _netApr(_apr, _protocolFee);
    uint256 interestEarned = _principalAmount.getInterest(netApr, block.timestamp - _startTime);
    uint256 fees = IFeeManager(getFeeManager).processFees(_received, 0);
    getCollectedFees += fees;
    _loanTermination(msg.sender, _loanId, _principalAmount, netApr, interestEarned, _received - fees);
}

Tools Used

Manual Review

Recommended Mitigation Steps

Consider checking and calling loanLiquidation() in settleWithBuyout() to ensure accurate accounting in the pool.

Assessed type

Other

c4-judge commented 7 months ago

0xA5DF marked the issue as primary issue

c4-judge commented 7 months ago

0xA5DF marked the issue as selected for report

c4-judge commented 7 months ago

0xA5DF marked the issue as satisfactory

0xend commented 7 months ago

Changing interest paid to use the end of the loan (this appears in another issue since this delta in time otherwise breaks teh maxSeniorRepayment concept). https://github.com/pixeldaogg/florida-contracts/pull/371