search

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

6 stars 6 forks source link

pkqs90 - Lender of the loan cannot perform `lenderCloseLoan()`/`setRepaymentListenerForBid()` after calling `claimLoanNFT()` #259

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

pkqs90

high

Lender of the loan cannot perform lenderCloseLoan()/setRepaymentListenerForBid() after calling claimLoanNFT()

Summary

For each active loan, there is a lender. The lender can call claimLoanNFT() to claim an NFT, and the lender of the loan would be the owner of the NFT. Then, the NFT can be transfered freely to other verified users to transfer the "lender" role of the loan. However, after a lender calls claimLoanNFT(), he cannot call some critical functions such as lenderCloseLoan() and setRepaymentListenerForBid().

Vulnerability Detail

After the lender calls claimLoanNFT(), the bid.lender turns to address(USING_LENDER_MANAGER) to mark that this loan should check the NFT owner as the lender. The contract provides a getLoanLender() function to return the real lender of the loan.

However, in setRepaymentListenerForBid() and _lenderCloseLoanWithRecipient(), it still checks against bid.lender, which is already set to address(USING_LENDER_MANAGER), forbidding the real lender to call these functions.

    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);
    }
    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);
        }
    }
    function setRepaymentListenerForBid(uint256 _bidId, address _listener)
        external
    {
        address sender = _msgSenderForMarket(bids[_bidId].marketplaceId);

        require(
            sender == bids[_bidId].lender,
>           "Only bid lender may set repayment listener"
        );

        repaymentListenerForBid[_bidId] = _listener;
    }
    function _lenderCloseLoanWithRecipient(
        uint256 _bidId,
        address _collateralRecipient
    ) 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");
    }

Impact

After a lender calls claimLoanNFT(), he nor future lender of the loan cannot perform lenderCloseLoan() or setRepaymentListenerForBid(), which are important features for the contract.

Code Snippet

Tool used

Manual review

Recommendation

Use getLoanLender() to get the real lender instead of bid.lender.

Duplicate of #11