Eggstravaganza

Summary

The EggVault.withdrawEgg function contains a potential reentrancy vulnerability due to its interaction with the ERC721 transferFrom method. While the function follows the proper checks-effects-interactions pattern by updating state variables before making external calls, the ERC721 standard includes a callback mechanism through onERC721Received that could still enable reentrancy attacks by malicious contract recipients.

Vulnerability Details

In the withdrawEgg function:

The vulnerability arises from the call to eggNFT.transferFrom(), which uses OpenZeppelin's standard ERC721 implementation. When transferring an NFT to a contract address, ERC721 calls _checkOnERC721Received, which executes the recipient's onERC721Received function.

If the msg.sender is a malicious contract with a specifically crafted onERC721Received function, it could call back into EggVault.withdrawEgg or other functions before the original transaction completes, potentially leading to unexpected behavior or exploitation.

Impact

While the current state variables (storedEggs and eggDepositors) are correctly updated before the external call, the vulnerability could still allow an attacker to:

1. Manipulate execution flow through callback functions

2. Potentially drain multiple NFTs if combined with other vulnerabilities

3. Cause denial of service or unexpected states in the contract system

The overall impact is medium-high as it requires specific conditions and crafted contracts to exploit but could result in unexpected behavior or loss of assets.

Tools Used

• Manual code review

• Slither

• Cross-contract interaction analysis (Claude)

Recommendations

The simplest and most robust solution is to use OpenZeppelin's ReentrancyGuard, as it's specifically designed to prevent this type of vulnerability with minimal code changes.