Eggstravaganza

First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

Mutable EggNFT Reference in EggVault Creates Risk of Permanent Asset Loss

Summary

The EggVault contract implements a mutable reference to the EggstravaganzaNFT contract via the setEggNFT function, which allows the owner to change the NFT contract address at any time without proper safeguards or migration mechanisms. This creates a risk of user assets becoming permanently inaccessible.

Vulnerability Details

The setEggNFT function allows the owner to change the NFT contract address without any restrictions:

function setEggNFT(address _eggNFTAddress) external onlyOwner {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}

The vulnerability stems from the vault's state management design. The contract maintains mappings (storedEggs and eggDepositors) that track deposited NFTs by their token IDs, but these mappings remain unchanged when the NFT contract reference is updated. This creates a critical state inconsistency between the vault's internal records and the actual NFT ownership. When users attempt to withdraw their NFTs after an address change, the withdrawEgg function will attempt to transfer tokens from the new NFT contract, which either doesn't have those tokens or assigns them to different owners.

Impact

  1. Users' deposited NFTs could become permanently locked in the vault

  2. The withdrawal function will fail for all previously deposited NFTs

  3. The system lacks synchronization between EggHuntGame and EggVault, creating a potential for permanent inconsistency

  4. The interaction between EggHuntGame and EggVault would break, as EggHuntGame has no mechanism to update its reference to the NFT contract

  5. Loss of user assets and trust in the platform

Proof of Concept:

  1. User A deposits an NFT (tokenId=1) into the vault

  2. The owner changes the eggNFT address to a new contract

  3. User A attempts to withdraw their NFT

  4. The withdrawal fails because:

    • The vault tries to transfer tokenId=1 from the new NFT contract

    • The new contract either doesn't have tokenId=1 or it belongs to a different user

    • The NFT remains locked in the vault with no way to retrieve it

  5. While restoring the original NFT address would make previously deposited NFTs accessible again (as failed withdrawal attempts revert the entire transaction), this depends on the owner's ability to recognize and rectify the issue promptly

Tools Used

Manual review

Recommendations

  1. Make the eggNFT address immutable after deployment to prevent any possibility of address changes:

EggstravaganzaNFT public immutable eggNFT;
constructor(address _eggNFTAddress) Ownable(msg.sender) {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
  1. If address changes must be supported, implement a proper migration mechanism:

    • Add a timelock for address changes to give users time to withdraw assets

    • Implement a function to migrate all deposited NFTs to the new contract

    • Create an emergency withdrawal function that works regardless of the current NFT address

  2. Add version control and compatibility checks:

    • Track NFT contract versions

    • Ensure new contracts maintain backward compatibility

    • Implement a coordinated upgrade process across all related contracts

  3. Add circuit breakers:

    • Implement a pause mechanism to halt deposits during migration

    • Add an emergency mode that allows only withdrawals during critical issues

Updates

Lead Judging Commences

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

State corruption

Changing the NFT contract address doesn't update the storedEggs and eggDepositors mappings

Support

FAQs

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