Eggstravaganza

Summary

The depositEgg() function in the EggVault contract is publicly callable, but it is clearly intended to be used only by the EggHuntGame contract. The game logic safely wraps the transfer and deposit in a single atomic transaction, protecting users from frontrunning.

However, by exposing depositEgg() publicly, users are unintentionally given a dangerous alternative: manually transferring their NFT and then calling depositEgg() themselves in a separate transaction. This opens the door for attackers to observe NFT transfers and frontrun the deposit call, effectively stealing the NFT and associating it with their own address in the vault.

Even though the intended and safe flow exists through the game contract, it is better practice to fully enforce this access control at the vault level, and not even allow users to call the function directly. This both prevents mistakes and protects against malicious exploitation.

Vulnerability Details

Intended safe flow (throug EggHuntGame):

unsafe flow:

1. user sends

2. plans to call :

3. Attacker frontruns and calls:

Impact

user asset theft, NFTs transferred manually to the vault can be stolen through frontrunning.

Tools Used

manual code analysis

Recommendations

Restrict depositEgg() so it can only be called by the game contract. More importantly, enforce this design intention in code, so users cannot call it directly, even by mistake: