Eggstravaganza
First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Reentrancy Risk in EggVault and EggHuntGame

zhouhuan19910202

Summary

The EggVault and EggHuntGame contracts are vulnerable to reentrancy attacks due to unprotected external calls to the EggstravaganzaNFT contract. This vulnerability allows an attacker to repeatedly execute critical functions before state updates occur, potentially draining NFTs from the Vault or disrupting deposit logic.

Vulnerability Details

The reentrancy vulnerabilities appear in the following functions:

  1. EggVault.withdrawEgg():

    solidity

    CollapseWrapCopy

    eggNFT.transferFrom(address(this), msg.sender, tokenId)``; storedEggs[tokenId] = false``; delete eggDepositors[tokenId]``;

    • The external call eggNFT.transferFrom occurs before state updates (storedEggs and eggDepositors), allowing a malicious NFT contract to reenter withdrawEgg.

  2. EggHuntGame.depositEggToVault():

    solidity

    CollapseWrapCopy

    eggNFT.transferFrom(msg.sender, address(eggVault), tokenId); eggVault.depositEgg(tokenId, msg.sender);

    • The eggNFT.transferFrom call could trigger a callback, potentially reentering the contract before eggVault.depositEgg updates the Vault state.

These issues arise because:

  • The EggstravaganzaNFT contract inherits from OpenZeppelin’s ERC721, which calls _beforeTokenTransfer. If overridden with malicious logic, it could reenter the calling contract.

  • No reentrancy protection (e.g., nonReentrant modifier) is present in either function.

An attacker can:

  • Deploy a malicious NFT contract mimicking EggstravaganzaNFT.

  • Trigger reentrancy by calling withdrawEgg or depositEggToVault, repeatedly executing the function before state is finalized.

Impact

This vulnerability has severe consequences:

  1. NFT Theft: In EggVault, an attacker could withdraw the same NFT multiple times, draining the Vault.

  2. State Corruption: In EggHuntGame, reentrancy during deposit could disrupt Vault synchronization, allowing unauthorized NFT control.

  3. Loss of Trust: Exploits could undermine confidence in the system, leading to player loss.

  4. Economic Damage: Stolen or duplicated NFTs could crash their market value.

Tools Used

  • Manual code review

  • Solidity compiler analysis (version ^0.8.23)

  • Theoretical attack simulation with reentrant contract patterns

Recommendations

  1. Add Reentrancy Protection:

    • Use OpenZeppelin’s ReentrancyGuard with the nonReentrant modifier.

  2. Update State Before External Calls:

    • Ensure state changes precede external interactions.

Example Implementation

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.23;
​
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "./EggstravaganzaNFT.sol";
​
contract EggVault is Ownable, ReentrancyGuard {
EggstravaganzaNFT public eggNFT;
mapping(uint256 => bool) public storedEggs;
mapping(uint256 => address) public eggDepositors;
​
event EggDeposited(address indexed depositor, uint256 tokenId);
event EggWithdrawn(address indexed withdrawer, uint256 tokenId);
​
constructor(address _eggNFTAddress) Ownable(msg.sender) {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
​
function withdrawEgg(uint256 tokenId) public nonReentrant {
require(storedEggs[tokenId], "Egg not in vault");
require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");
​
// Update state first
storedEggs[tokenId] = false;
delete eggDepositors[tokenId];
​
// External call after state update
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}
​
// ... other functions ...
}
​
// EggHuntGame modification
contract EggHuntGame is Ownable, ReentrancyGuard {
function depositEggToVault(uint256 tokenId) external nonReentrant {
require(eggNFT.ownerOf(tokenId) == msg.sender, "Not owner of this egg");
eggNFT.transferFrom(msg.sender, address(eggVault), tokenId);
eggVault.depositEgg(tokenId, msg.sender);
}
​
// ... other functions ...
}
Updates

Lead Judging Commences

m3dython Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.