Bauer - Use safeMint instead of mint for ERC721

[sherlock-admin]

Bauer

medium

Use safeMint instead of mint for ERC721

Summary

Use safeMint instead of mint for ERC721

Vulnerability Detail

The msg.sender will be minted as a proof of staking NFT when the claimLoanNFT() function is called.
However, if lender is a contract address that does not support ERC721, the NFT can be frozen in the contract.

As per the documentation of EIP-721:
A wallet/broker/auction application MUST implement the wallet interface if it will accept safe transfers.

Ref: https://eips.ethereum.org/EIPS/eip-721
As per the documentation of ERC721.sol by Openzeppelin
Ref: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L274-L285

LenderManager.sol
   function registerLoan(uint256 _bidId, address _newLender)
        public
        override
        onlyOwner
    {
        _mint(_newLender, _bidId);
    }

Impact

Users possibly lose their NFTs

Code Snippet

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/LenderManager.sol#L45

Tool used

Manual Review

Recommendation

Use safeMint instead of mint to check received address support for ERC721 implementation.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L262

[securitygrid]

Escalate for 10 USDC
This is valid low/info. Lender's own fault. safeMint can introduce reentrancy issues.

[sherlock-admin]

Escalate for 10 USDC
This is valid low/info. Lender's own fault. safeMint can introduce reentrancy issues.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ethereumdegen]

Github PR: Issue 8 - Use SafeMint

[hrishibhat]

Escalation accepted

Valid low
Users unable to claim/mint ERC721 if they do not have the necessary implementations if the protocol design clearly expects users to be able to is a valid low issue. Also as pointed out, safeMint can always add more issues to the code.
The same will be updated in Sherlock rules.

[sherlock-admin]

Escalation accepted

Valid low
Users unable to claim/mint ERC721 if they do not have the necessary implementations if the protocol design clearly expects users to be able to is a valid low issue. Also as pointed out, safeMint can always add more issues to the code.
The same will be updated in Sherlock rules.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.