Eggstravaganza

Summary

• The withdrawEgg function in EggVault.sol performs an external call to transferFrom after updating state, potentially exposing the contract to reentrancy attacks if the NFT contract or receiver has malicious logic.

Vulnerability Details

• The function:

• State changes (storedEggs[tokenId] = false and delete eggDepositors[tokenId]) occur before the external call to eggNFT.transferFrom.

• If the NFT contract or the receiver (msg.sender) is malicious, it could reenter withdrawEgg before the state updates are fully processed, potentially allowing multiple withdrawals of the same egg.

Impact

• An attacker could drain the vault of multiple eggs if reentrancy is exploited, especially if the NFT contract allows callbacks (e.g., via onERC721Received).

• This could lead to loss of assets and disruption of the vault’s integrity.

Tools Used

• Manual code review.

• Knowledge of reentrancy vulnerabilities (e.g., SWC-107: Reentrancy).

Recommendations

• Follow the Checks-Effects-Interactions pattern: perform all external calls after state changes are complete. However, since state changes here occur before the transfer, add a reentrancy guard.

• Use OpenZeppelin’s ReentrancyGuard

• Alternatively, verify that EggstravaganzaNFT (an ERC721) doesn’t trigger callbacks, but a guard is still safer for future-proofing.