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

Missing Vault Registration Leads to Permanently Locked NFTs

howiecht

Summary

Participants who have obtained eggs through the game's search mechanism can transfer their NFT eggs directly to the vault using standard ERC721 transfer methods. However, these directly transferred eggs cannot be withdrawn as they aren't properly registered in the vault's storage. This results in eggs becoming permanently locked in the vault with no possibility of retrieval.

Vulnerability Details

When users transfer their egg NFTs directly to the vault using transferFrom() instead of using the game's depositEggToVault() function, the eggs arrive at the vault contract but the critical registration step is bypassed. The vault keeps track of deposited eggs using the storedEggs mapping and records the original depositor in the eggDepositors mapping. Direct transfers skip this registration process.

Since the withdrawal function requires that eggs be properly registered through the storedEggs mapping (require(storedEggs[tokenId], "Egg not in vault")), directly transferred eggs cannot be withdrawn, despite physically being in the vault's possession.

function testDirectTransferToVault() public {
// Start the game with a duration.
uint256 duration = 200;
game.startGame(duration);
​
// Set threshold to 100 to guarantee that an egg is always found.
game.setEggFindThreshold(100);
​
// Alice attempts to search for an egg.
vm.startPrank(alice);
game.searchForEgg();
uint256 eggCounter = game.eggCounter();
​
nft.transferFrom(alice, address(vault), eggCounter);
​
vm.expectRevert("Egg not in vault");
vault.withdrawEgg(eggCounter); // egg stuck in vault
vm.stopPrank();
assertEq(nft.ownerOf(eggCounter), address(vault));
}

Impact

This vulnerability leads to permanent asset loss for users who inadvertently transfer their eggs directly to the vault. There is no recovery mechanism for these assets, and they become locked in the vault contract forever, reducing the total supply of active egg NFTs and causing financial loss to affected users.

Tools Used

  • Manual code review

  • Foundry testing framework to validate the vulnerability

Recommendations

Implement the onERC721Received hook to properly register eggs that are transferred directly to the vault:

function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external override returns (bytes4) {
require(msg.sender == address(eggNFT), "Not from the NFT contract");
// Register the egg and its depositor
storedEggs[tokenId] = true;
eggDepositors[tokenId] = from;
emit EggDeposited(tokenId, from);
return IERC721Receiver.onERC721Received.selector;
}

Then add a rescue function that allows contract administrators to help users recover accidentally transferred eggs:

function rescueUnregisteredEgg(uint256 tokenId, address recipient) external onlyOwner {
require(eggNFT.ownerOf(tokenId) == address(this), "Egg not in vault");
require(!storedEggs[tokenId], "Egg is registered");
eggNFT.transferFrom(address(this), recipient, tokenId);
emit EggRescued(tokenId, recipient);
}
Updates

Lead Judging Commences

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

Unsafe direct token transfer

Users can transfer NFTs directly to the vault using standard ERC721 transferFrom(), bypassing the registration

Appeal created

mishoko Auditor
about 2 months ago
m3dython Lead Judge
about 2 months ago
m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Unsafe direct token transfer

Users can transfer NFTs directly to the vault using standard ERC721 transferFrom(), bypassing the registration

Support

FAQs

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