Eggstravaganza

Description:
EggVault::depositEgg is a public function, allowing users to deposit their eggs directly into the EggVault contract instead of doing this through the EggHuntGame. The function only checks if the egg has been transferred to the EggVault contract. So whoever calls the function first will claim the egg after the real owner deposited it. A malicious user might watch for the ERC721::Transfer event and call the EggVault::depositEgg function before the real owner does and steal the egg. You can read more about listening to events.

Impact:
Real EggstravaganzaNFT egg owners will get their NFT stolen.

Proof of Concept:

1. Users finds and mints an egg NFT.

2. User transfers their egg NFT to the vault contract directly.

3. A hacker watches for the transfer event and deposits the egg himself before the real owner does.

4. User tries to deposit the egg, but can't and finds out it was already deposited and stolen.

Proof of Code:
Place the following test inside the EggGameTest:

Recommended Mitigation:

1. You can change the EggVault::depositEgg function to include the egg transfer as well so the transfer and deposit will happen in one transaction like the EggHuntGame::depositEggToVault function does. Of course, now the user will approve the EggVault instead of the EggHuntGame to transfer the NFT.

2. If you want to keep the original EggHuntGame::depositEggToVault functionality, you can still use mitigation #1 as a separate function to allow direct deposits and have the EggVault::depositEgg function only be callable from an approved mapping of EggHuntGames inside the EggVault or use openzeppelin's access control.