Eggstravaganza

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

Non-Atomic Deposit Flow in EggHuntGame

Summary

The EggHuntGame.depositEggToVault() performs an NFT transfer followed by a call to vault.depositEgg(). This 2-step process introduces a non-atomic flow that can be front-run or interrupted.

Vulnerability Details

Since transferFrom() is executed before calling the vault's deposit function, an attacker could detect the transfer and quickly call depositEgg() themselves, stealing the depositor slot. This is particularly relevant in a public mempool environment.

Impact

  • Race condition between NFT transfer and deposit registration.

  • Users could lose access to their own NFTs.

Tools Used

  • Manual code inspection

  • Mempool frontrunning logic reasoning

Recommendations

/// @notice Allows a player to deposit their egg NFT into the Egg Vault.
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);
+ eggNFT.safeTransferFrom(msg.sender, address(eggVault), tokenId);
}
  • Use safeTransferFrom() instead.

  • Let the vault handle depositor registration via onERC721Received().

  • This ensures atomic deposit + state update in one transaction.

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.