Summary

The depositEgg() function in EggVault.sol performs an external call to the NFG contract (eggNFT.ownerOf(...)) before updating its internal state. This violates the checks-effects-interactions pattern and exposes the contract to potential reentrancy vulnerabilities if a malicious ERC721 contract is used in place of the trusted one currently hardcoded.

This isn't immediately exploitable in the current setup, but the flaw represents a critical design issue that can be abused if the vault is ever used in a more generalized setting.

Vulnerability Details

In EggVault.sol, the depositEgg() function is responsible for verifying ownership of an NFT and recording its deposit:

The first line performs an external call to eggNFT.ownerOf(...) before updating internal state (storedEggs[tokenId], eggDepositors[tokenId]). If eggNFTwere a malicious contract, theownerOf()function could be overridden to perform reentrant calls back intodepositEgg()`, exploiting the fact that the internal state has not yet been updated.

This violates the checks-effects-interactions pattern, which dictates that all internal state changes should occur before any external interactions, to minimize attack surfaces.

Impact

If the eggVault contract were used with an untrusted ERC721 token, a malicious contract could override the ownerOf() function and reenter the vault during deposit, leading to:

• Duplicate deposits

• Event spoofing

• Storage corruption

• Logic-based denial-of-service attacks

This pattern has been exploited in various high-profile reentrancy attacks like The DAO and Parity Multisig.

Even though it is not currently exploitable in the current setup (since eggNFT is a trusted OpenZeppelin based contract), this flaw introduces critical risk in future use cases where the NFT contract is user-supplied, upgradeable, or unverified.

Tools Used

• Manual review

• Aderyn

Recommendations

To mitigate the issue:

1. Apply the checks-effects-interactions pattern - perform all internal state updates before calling eggNFT.ownerOf(...).

1. Alternatively, cache the result of the external call first and validate after effects.

2. Add a reentrancy guard (nonReentrant) as a secondary safety net.
   4, Enforce stricter controls on the type of contracts that can be assigned as eggNFT.