aman - After Claiming NFT the lender will not be able to close Loan

[sherlock-admin2]

aman

high

After Claiming NFT the lender will not be able to close Loan

Summary

The Protocol support that the loan ownership can be transfer, to achieve this feature they provided solution to mint NFT against BidId of that loan and set the USING_LENDER_MANAGER as a lender. the issue is the owner of loan is not be able to close the loan if its get defaulted and there is no way to get the ownership back from USING_LENDER_MANAGER.

Vulnerability Detail

The Alice (Lender) accept the bid after some time he relise that he shulod transfer the ownership of loan to Bob, for this he minted the NFT. so upon minting the NFT the protocol mint the nft to Bob and set the USING_LENDER_MANAGER as bid.lender :

function claimLoanNFT(uint256 _bidId)
        external
        acceptedLoan(_bidId, "claimLoanNFT")
        whenNotPaused
    {
        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); // @audit : lender address updated

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

Bob receive the NFT after some time the Loan get Defaulted so now the Bob wants to Close the loan and get back the assets locked. here Bob should call lenderCloseLoanWithRecipient or CloseLoan function in both of cases _lenderCloseLoanWithRecipient this function get called:

    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"); // @audit : claim NFT will also effect this
        collateralManager.lenderClaimCollateral(_bidId);

        emit LoanClosed(_bidId);
    }

As the lender is already changed so the Bob can not close the loan.

POC

function test_revert_claim_loan_nft_close_loan() public {
        // @audit : nft owner can be replicated 
        uint256 bidId = 1;
        setMockBid(bidId);

        tellerV2.setLenderManagerSuper(address(lenderManagerMock));

        tellerV2.mock_setBidState(bidId, BidState.ACCEPTED);

        tellerV2.setMockMsgSenderForMarket(address(lender));
        vm.prank(address(lender));

        tellerV2.claimLoanNFT(bidId);
        address owener1 = lenderManagerMock.ownerOf(1);
        vm.warp(block.timestamp + 100 days);
        vm.prank(address(lender));
        vm.expectRevert();
        tellerV2.lenderCloseLoanWithRecipient(bidId, address(this));
        //assert
    }

add this to tests/TellerV2/TellerV2Bid.sol file and run with command : forge test --mt test_revert_claim_loan_nft_close_loan -vvv

Impact

After Claiming NFT the Lender can not perform Close Loan Operation.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L590

Tool used

Manual Review

Recommendation

use getLoanLender() to get the owner of NFT if minted the require check should be like follow :

        require(sender == bid.lender || sender == getLoanLender(bidId) , "only lender can close loan"); // @audit : claim NFT will also effect this

Duplicate of #11