Eggstravaganza

Summary

The EggVault::depositEgg() function lacks access control, allowing anyone to mark an NFT as "deposited" if it has been transferred to the vault. This enables malicious actors to steal deposited NFTs by front-running legitimate deposits or exploiting direct transfers to the vault.

Vulnerability Details

Location:
EggVault::depositEgg(uint256 tokenId, address depositor)

Attack Scenarios:

1. Front-Running Legitimate Deposits

   • A user approves and calls EggHuntGame::depositEggToVault(tokenId).

   • An attacker monitors the mempool, sees the pending transferFrom, and front-runs it with their own depositEgg(tokenId, attackerAddress).

   • The attacker becomes the "depositor" and can later withdraw the NFT.

2. Direct Transfer Exploit

   • If a user accidentally (or intentionally) sends an NFT directly to the vault via transferFrom(user, vault, tokenId), anyone can call depositEgg() to claim ownership.

Root Cause:

• The function does not verify if the caller is authorized EggHuntGame contract.

• It only checks if the NFT is in the vault (eggNFT.ownerOf(tokenId) == address(this)), which is insufficient for security.

Impact

Loss of NFTs for legitimate users since attackers can irreversibly claim deposited NFTs.

Tools Used

Manual Review: Identified missing access control.

Recommendations

####*1. Restrict depositEgg to EggHuntGame Only
Add a modifier to ensure only the game contract can call depositEgg: