`LensBaseERC721.sol` does not implement `_safeMint()` and the case where `_safeMint()` should be used rather than `_mint()` wherever possible is missed by bot.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/LensHub.sol#L89-L102

Vulnerability details

Impact

Users possibly lose their NFTs

Proof of Concept

• According to the M-06 of the bot race _safeMint() should be used rather than _mint() wherever possible. It caught 04 cases and missed the case in LensHub.sol#createProfile()

  File: 2023-07-lens/contracts/LensHub.sol
  98:            _mint(createProfileParams.to, profileId);

• LensBaseERC721.sol only implements _mint() and does not implement _safeMint()
• As per the documentation of EIP-721:

  ''A wallet/broker/auction application MUST implement the wallet interface if it will accept safe transfers.''

• As per the documentation of ERC721.sol by Openzeppelin:

  /**
   * @dev Mints `tokenId` and transfers it to `to`.
   *
   * WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible
   *
   * Requirements:
   *
   * - `tokenId` must not exist.
   * - `to` cannot be the zero address.
   *
   * Emits a {Transfer} event.
   */
  function _mint(address to, uint256 tokenId) internal virtual {

Tools Used

Manual Review

Recommended Mitigation Steps

1. Implement _safeMint() in LensBaseERC721.sol : References
2. Use _safeMint instead of _mint to check received address support for ERC721 implementation.

Assessed type

ERC721

[c4-pre-sort]

141345 marked the issue as duplicate of #17

[c4-pre-sort]

141345 marked the issue as not a duplicate

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-sponsor]

donosonaumczuk marked the issue as disagree with severity

[donosonaumczuk]

We were aware of this and we are considering it. We think this should be QA/Low.

[Picodes]

Downgrading to Low as there is already a whitelist, and using SafeMint is in my opinion more an additional safety check that a security vulnerability

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-c