onERC721Received callback is never called when new tokens are minted.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PowerFarmNFTs/PowerFarmNFTs.sol#L66-L77

Vulnerability details

Impact

After minting is completed OnERC721Received() is not called to verify if the target address which is the msg.sender implements {IERC721Receiver-onERC721Received}.The ERC721 implementation used by the Minter Reserver contract does not properly call the corresponding callback when new tokens are minted. However, if key owner is a contract address that does not support ERC721, the NFT can be frozen in the contract as per the documentation of the EIP721. Therefore calling mint this way does not ensure that the receiver of the NFT is able to accept them, _safeMint() should be used with reentrancy guards as a guard to protect the user as it checks to see if a user can properly accept an NFT and reverts otherwise.

Proof of Concept

As a result of the mintKey() not implementing the onERC71Received callback correctly when tokens are minted, it can lead to tokens being stuck in the target address if it is a contract.

     function mintKey(
        address _keyOwner,
        uint256 _keyId
    )
        external
        onlyFarmContract
    {
        _mint(
            _keyOwner,
            _keyId
        );
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Make use of Safemint instead of mint to prevent NFTs from being stuck and also add a nonReentrant modifier so as to prevent a reentrancy possibility

Assessed type

ERC721

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[GalloDaSballo]

Not a real vulnerability

[trust1995]

As per Supreme court rulings abundance of caution cannot be submitted as Med severity

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid