Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Use of `transferFrom` over `safeTransferFrom()` in NFT transfers

0xgo4ko

Root + Impact

Description

The BidBeastsNFTMarketPlace contract utilizes the transferFrom method to transfer NFTs, which is not the recommended approach for ERC721 tokens. The transferFrom method does not perform any checks to ensure that the recipient is capable of receiving the NFT.

If the recipient is a contract that does not implement the required ERC721 interface, the NFT could be permanently locked, as it would not be able to be transferred out of that contract.

function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
...
BBERC721.transferFrom(msg.sender, address(this), tokenId);
...
}
​
function unlistNFT(uint256 tokenId) external isListed(tokenId) isSeller(tokenId, msg.sender) {
...
BBERC721.transferFrom(address(this), msg.sender, tokenId);
...
}
​
function _executeSale(uint256 tokenId) internal {
...
BBERC721.transferFrom(address(this), bid.bidder, tokenId);
...
}

Recommended Mitigation

To enhance the safety of NFT transfers, it is advisable to use the safeTransferFrom method instead of transferFrom.

Updates

Lead Judging Commences

cryptoghost Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts Marketplace: Risk of Locked NFTs

Non-safe transferFrom calls can send NFTs to non-compliant contracts, potentially locking them permanently.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!