Lenders might not be able to close their loans and get their collateral back in the case of default

Summary

Lenders might not be able to close their loans and get their collateral back in the case of default because of a state change upon a lender claiming a loan NFT.

Vulnerability Detail

Lenders can call the claimLoanNFT() function in order to mint a loan NFT.

    function claimLoanNFT(uint256 _bidId)
        external
        acceptedLoan(_bidId, "claimLoanNFT")
        whenNotPaused
    {
        // Retrieve bid
        Bid storage bid = bids[_bidId];

        address sender = _msgSenderForMarket(bid.marketplaceId);
        require(sender == bid.lender, "only lender can claim NFT");

        // set lender address to the lender manager so we know to check the owner of the NFT for the true lender
        bid.lender = address(USING_LENDER_MANAGER);

        // mint an NFT with the lender manager
        lenderManager.registerLoan(_bidId, sender);
    }

This changes the bid.lender variable to the USING_LENDER_MANAGER address. The comment above the state change states that they should check the owner of the NFT for the actual lender. That is the reason they have a special getter function getLoanLender():

 function getLoanLender(uint256 _bidId)
        public
        view
        returns (address lender_)
    {
        lender_ = bids[_bidId].lender;

        if (lender_ == address(USING_LENDER_MANAGER)) {
            return lenderManager.ownerOf(_bidId);
        }

        //this is left in for backwards compatibility only
        if (lender_ == address(lenderManager)) {
            return lenderManager.ownerOf(_bidId);
        }
    }

The issue is that this function is not used when a lender tries to close his loan when it is defaulted. A lender calls lenderCloseLoan():

function lenderCloseLoan(uint256 _bidId)
        external
        acceptedLoan(_bidId, "lenderClaimCollateral")
    {
        Bid storage bid = bids[_bidId];
        address _collateralRecipient = bid.lender;

        _lenderCloseLoanWithRecipient(_bidId, _collateralRecipient);
    }

The function uses the bid.lender as the _collateralRecipient. Then, upon calling the internal _lenderCloseLoanWithRecipient(), the mistake is done again as it is required that the sender is the bid.lender which is not the case as the bid.lender was changed upon claiming the loan NFT.

function _lenderCloseLoanWithRecipient(
        uint256 _bidId,
        address _collateralRecipient // @audit _collateralRecipient is not used
    ) internal acceptedLoan(_bidId, "lenderClaimCollateral") {
        require(isLoanDefaulted(_bidId), "Loan must be defaulted.");

        Bid storage bid = bids[_bidId];
        bid.state = BidState.CLOSED;

        address sender = _msgSenderForMarket(bid.marketplaceId);
        require(sender == bid.lender, "only lender can close loan");

        collateralManager.lenderClaimCollateral(_bidId);

        emit LoanClosed(_bidId);
    }

Impact

Lender with a loan that has defaulted will not be able to get the collateral.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L745-L774 https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L578-L594

Tool used

Manual Review

Recommendation

Use getLoanLender() instead.

Duplicate of #11

sherlock-audit / 2024-04-teller-finance-judging

samuraii77 - Lenders might not be able to close their loans and get their collateral back in the case of default #208