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

Missing NFT Transfer Approval Check

zhouhuan19910202

Summary

The depositEggToVault function in the EggHuntGame contract fails to verify approval status before attempting to transfer NFTs, potentially leading to failed transactions and user experience issues.

Vulnerability Details

The current depositEggToVault function does not check if the game contract has permission to transfer the player's NFT:

function depositEggToVault(uint256 tokenId) external {
require(eggNFT.ownerOf(tokenId) == msg.sender, "Not owner of this egg");
// The player must first approve the transfer on the NFT contract.
eggNFT.transferFrom(msg.sender, address(eggVault), tokenId);
eggVault.depositEgg(tokenId, msg.sender);
}

The comment mentions that players must first approve the transfer, but the code does not verify this. If players haven't approved beforehand, the transferFrom call will fail with an unclear error message.

Impact

  1. User experience issues - transactions may fail due to missing approvals

  2. Unclear error messages leading to user confusion

  3. Increased user support and education costs

Tools Used

  • Manual code review

  • ERC721 standard compliance analysis

Recommendations

Add explicit approval checks before attempting to transfer the NFT:

function depositEggToVault(uint256 tokenId) external {
require(eggNFT.ownerOf(tokenId) == msg.sender, "Not owner of this egg");
// Check if the game contract is approved to transfer this NFT
address approved = eggNFT.getApproved(tokenId);
bool isApprovedForAll = eggNFT.isApprovedForAll(msg.sender, address(this));
require(approved == address(this) || isApprovedForAll, "Game not approved to transfer this egg");
// Transfer NFT to vault
eggNFT.transferFrom(msg.sender, address(eggVault), tokenId);
eggVault.depositEgg(tokenId, msg.sender);
}
Updates

Lead Judging Commences

m3dython Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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