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

Unsafe NFT Transfer Pattern in Deposit Function

albert

Summary

The depositEggToVault() function uses an insecure transfer pattern that could lead to NFT loss if the vault deposit fails after transfer.

Vulnerability Details

function depositEggToVault(uint256 tokenId) external {
eggNFT.transferFrom(msg.sender, address(eggVault), tokenId); // ❌ Irreversible transfer
eggVault.depositEgg(tokenId, msg.sender); // Potentially failing operation
}

If the vault's depositEgg() call fails due to:

  • Temporary vault pause

  • Gas limit exceeded

  • Reentrancy protection
    The NFT remains stranded in the vault contract without being properly registered.

Impact

• Permanent loss of NFTs

  • Requires admin intervention to recover
    • Erodes user trust in protocol safety

Tools Used

• Manual code review
• Slither (detects dangerous external calls)
• Foundry test simulating failed deposit:

function testFailedDeposit() public {
vault.pause();
vm.expectRevert("Vault paused");
game.depositEggToVault(1); // NFT lost in vault
}

Recommendations

Step 1: Modify Deposit Function (EggHuntGame.sol)

function depositEggToVault(uint256 tokenId) external {
// Use safeTransferWithData
eggNFT.safeTransferFrom(
msg.sender,
address(eggVault),
tokenId,
abi.encode(msg.sender) // Encode depositor address
);
}

Step 2: Implement Callback (EggVault.sol)

// ERC721 Receiver Implementation
function onERC721Received(
address,
address from,
uint256 tokenId,
bytes memory data
) external override returns (bytes4) {
// Validate NFT contract
require(msg.sender == address(eggNFT), "Unauthorized NFT");
// Decode depositor address
address depositor = abi.decode(data, (address));
// Execute deposit
_depositEgg(tokenId, depositor);
return this.onERC721Received.selector;
}
​
// Internal deposit logic
function _depositEgg(uint256 tokenId, address depositor) internal {
require(!storedEggs[tokenId], "Already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor;
emit EggDeposited(depositor, tokenId);
}
Updates

Lead Judging Commences

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

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.