Eggstravaganza
First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Unsafe NFT transferfrom and mint

sharkeateateat

Summary

The EggHuntGame and EggstravaganzaNFT contract uses unsafe NFT transferFrom methods (_mint and transferFrom ) instead of their safe counterparts (_safeMint and safeTransferFrom ). This can lead to permanent loss of NFTs when they are transferred to contracts that don't support ERC721 token reception.

Vulnerability Details

The contract implements NFT transfers in two critical functions:

  1. In the EggstravaganzaNFT::mintEgg external function:

function mintEgg(address to, uint256 tokenId) external returns (bool) {
require(msg.sender == gameContract, "Unauthorized minter");
@> _mint(to, tokenId);
totalSupply += 1;
return true;
}

2.In the EggHuntGame::depositEggToVaultexternal function:

function depositEggToVault(uint256 tokenId) external {
require(eggNFT.ownerOf(tokenId) == msg.sender, "Not owner of this egg");
@> eggNFT.transferFrom(msg.sender, address(eggVault), tokenId);
eggVault.depositEgg(tokenId, msg.sender);
}

The vulnerability arises because these functions don't verify whether the recipient can handle ERC721 tokens. According to the ERC721 standard, contracts must implement the onERC721Received function to receive tokens. Without this check, tokens can be permanently locked in contracts that don't support ERC721.

Impact

  1. Permanent loss of NFTs when transferred to incompatible contracts

  2. No recovery mechanism for lost tokens

  3. Potential financial loss for users who accidentally send NFTs to contract addresses

  4. Deterioration of user trust in the platform

Tools Used

Manual review

Recommendations

Replace unsafe transfer methods with their safe counterparts:

function mintEgg(address to, uint256 tokenId) external returns (bool) {
require(msg.sender == gameContract, "Unauthorized minter");
- _mint(to, tokenId);
+ _safeMint(to, tokenId);
totalSupply += 1;
return true;
}
function depositEggToVault(uint256 tokenId) external {
require(eggNFT.ownerOf(tokenId) == msg.sender, "Not owner of this egg");
- eggNFT.transferFrom(msg.sender, address(eggVault), tokenId);
+ eggNFT.safetransferFrom(msg.sender, address(eggVault), tokenId);
eggVault.depositEgg(tokenId, msg.sender);
}
Updates

Lead Judging Commences

m3dython Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Unsafe ERC721 Minting

Protocol doesn't check if recipient contracts can handle ERC721 tokens

Unsafe ERC721 Transfer

NFTs are transferred to contracts without onERC721Received implementation.

Support

FAQs

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

Give us feedback!