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

0 stars 0 forks source link

settleWithBuyout() lack of call LoanManager.loanRepayment() #9

Closed c4-bot-3 closed 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-L94

Vulnerability details

Vulnerability details

in AuctionWithBuyoutLoanLiquidator The lender who has lent out the most can purchase the NFT through settleWithBuyout()

    function settleWithBuyout(
        address _nftAddress,
        uint256 _tokenId,
        Auction calldata _auction,
        IMultiSourceLoan.Loan calldata _loan
    ) external nonReentrant {
        /// TODO: Originator fee
        _checkAuction(_nftAddress, _tokenId, _auction);
        uint256 timeLimit = _auction.startTime + _timeForMainLenderToBuy;
        if (timeLimit < block.timestamp) {
            revert OptionToBuyExpiredError(timeLimit);
        }
        uint256 largestTrancheIdx;
        uint256 largestPrincipal;
        for (uint256 i = 0; i < _loan.tranche.length;) {
            if (_loan.tranche[i].principalAmount > largestPrincipal) {
                largestPrincipal = _loan.tranche[i].principalAmount;
                largestTrancheIdx = i;
            }
            unchecked {
                ++i;
            }
        }
        if (msg.sender != _loan.tranche[largestTrancheIdx].lender) {
            revert NotMainLenderError();
        }
        ERC20 asset = ERC20(_auction.asset); 
        uint256 totalOwed;
        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);
@>              //@audit miss notice LoanManager if lender is LoanManager
            }
            unchecked {
                ++i;
            }
        }
        IMultiSourceLoan(_auction.loanAddress).loanLiquidated(_auction.loanId, _loan);

        asset.safeTransfer(_auction.originator, totalOwed.mulDivDown(_auction.triggerFee, _BPS));

        ERC721(_loan.nftCollateralAddress).transferFrom(address(this), msg.sender, _tokenId);

        delete _auctions[_nftAddress][_tokenId];

        emit AuctionSettledWithBuyout(_auction.loanAddress, _auction.loanId, _nftAddress, _tokenId, largestTrancheIdx);
    }

The above code repays other lenders, but it lacks notice the lender If the lender is a LoanManager, notify it to accounting using ILoanManager(tranche.lender).loanRepayment() like in MultiSourceLoan or LiquidationDistributor.

It's crucial to notify the LoanManager for accounting. Failure to do so will result in an incorrect allocation of repaid assets to the correct WithdrawalQueue.

Impact

The lack of notification to the LoanManager for accounting will result in incorrect asset allocation.

Recommended Mitigation

suggest:

  1. add getLoanManagerRegistry to contract
  2. call loanRepayment() in settleWithBuyout()
    function settleWithBuyout(
        address _nftAddress,
        uint256 _tokenId,
        Auction calldata _auction,
        IMultiSourceLoan.Loan calldata _loan
    ) external nonReentrant {
...
        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);

+               if (getLoanManagerRegistry.isLoanManager(tranche.lender)) {
+                   ILoanManager(thisTranche.lender).loanRepayment(
+                       thisTranche.loanId,
+                       thisTranche.principalAmount,
+                       thisTranche.aprBps,
+                       thisTranche.accruedInterest,
+                       _loan.protocolFee,
+                       thisTranche.startTime
+                   );
+               }

            }
            unchecked {
                ++i;
            }
        }
....
    }

Assessed type

Context

c4-judge commented 7 months ago

0xA5DF marked the issue as duplicate of #49

c4-judge commented 7 months ago

0xA5DF marked the issue as satisfactory