Eggstravaganza

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

NFT Contract Address Update Risk in EggVault Contract

Summary

The EggVault contract contains a significant vulnerability in its design: the contract owner can change the NFT contract address via the setEggNFT function at any time without adequate safeguards. This functionality creates a critical risk where previously deposited NFTs could become permanently locked in the vault if the NFT contract reference is altered. Users who have deposited assets would be unable to withdraw their NFTs as the contract would attempt to interact with a different NFT contract than the one their tokens reside in.

Vulnerability Details

The vulnerable function is found in the EggVault contract:

/// @notice Set the NFT contract address.
function setEggNFT(address _eggNFTAddress) external onlyOwner {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}

This function allows the owner to change the reference to the EggstravaganzaNFT contract at any time. The issue stems from the fact that:

  1. There is no initialization check to prevent changing the address after deposits have been made

  2. No migration mechanism exists to handle existing deposits when changing the contract address

  3. The withdrawal function relies on the current NFT address reference:

function withdrawEgg(uint256 tokenId) public {
require(storedEggs[tokenId], "Egg not in vault");
require(eggDepositors[tokenId] == msg.sender, "Not the original depositor");
storedEggs[tokenId] = false;
delete eggDepositors[tokenId];
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}

If the NFT contract address is changed after users have already deposited NFTs, those NFTs would remain in the old contract while the withdrawal function would attempt to call transferFrom on the new contract. This would fail, permanently locking user assets in the vault.

Impact

  1. Permanent Asset Loss: Users' NFTs could become permanently locked in the vault if the NFT contract address is changed.

  2. Trust Violation: Users deposit assets with the expectation of being able to withdraw them later, but this mechanism can be broken unilaterally by the owner.

Tools Used

Manual Review

Recommendations

Immutable NFT Contract Reference: Make the NFT contract reference immutable by setting it only in the constructor:

constructor(address _eggNFTAddress) Ownable(msg.sender) {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
// Remove the setEggNFT function entirely
Updates

Lead Judging Commences

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Appeal created

edoscoba Submitter
4 months ago
m3dython Lead Judge 4 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.