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

`EggVault::eggNFT` address can be changed by the owner, potentially locking deposited NFTs

mirkopezo

Summary

The EggVault contract allows the owner to change the address of the associated EggstravaganzaNFT contract at any time after deployment using the setEggNFT function. If the owner sets this to an incorrect address, all NFTs previously deposited in the vault become permanently irrecoverable.

Vulnerability Details

The EggVault contract stores the address of the EggstravaganzaNFT contract in the eggNFT state variable. This variable is initialized to address(0) after deployment and can be updated later by the owner via the setEggNFT function:

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

The withdrawEgg function uses this eggNFT variable to transfer the NFT back to the depositor:

function withdrawEgg(uint256 tokenId) public {
// ... checks ...
eggNFT.transferFrom(address(this), msg.sender, tokenId); // Uses the potentially changed eggNFT address
// ... state updates ...
}

If an owner calls setEggNFT after users have deposited NFTs, changing eggNFT to a different address (either maliciously or accidentally), the eggNFT.transferFrom(...) call within withdrawEgg will target the new contract address. This call will fail because the new contract address doesn't hold the NFTs or recognize the vault as the owner/operator for those specific tokenIds. Consequently, the withdrawal transaction will revert, effectively locking the deposited NFTs in the vault contract forever.

Given that the EggstravaganzaNFT contract is likely deployed before the EggVault, there is no apparent need for the eggNFT address to be mutable.

Impact

Users who deposited their NFTs into the vault can permanently lose access to their assets if the owner changes the eggNFT address. The withdrawEgg function will become unusable for all previously deposited NFTs.

Tools Used

Manual Review

Recommendations

Initialize the eggNFT address immutably in the constructor. This ensures the correct NFT contract is associated with the vault from the beginning and cannot be changed later, preventing the potential locking of user funds.

  1. Remove the setEggNFT function.

  2. Modify the constructor to accept the _eggNFTAddress and initialize the eggNFT variable there. Make the eggNFT variable immutable.

contract EggVault is Ownable {
/// @notice Reference to the EggstravaganzaNFT contract.
EggstravaganzaNFT public immutable eggNFT; // Make immutable
​
// ... other state variables ...
​
constructor(address _eggNFTAddress) Ownable(msg.sender) {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}
​
// Remove the setEggNFT function entirely
​
// ... rest of the contract ...
}
Updates

Lead Judging Commences

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

Support

FAQs

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