Eggstravaganza

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

NFTs can be permanently locked in vault if deposited with the zero address

Summary

The depositEgg function allows depositing an NFT into the EggVault contract and associating it with a depositor address. However, it lacks a check to prevent the depositor address from being the zero address (address(0)).

Vulnerability Details

The depositEgg function accepts a tokenId and a depositor address as parameters. It updates the eggDepositors mapping to link the tokenId with the provided depositor.

// src/EggVault.sol
function depositEgg(uint256 tokenId, address depositor) public {
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor; // 'depositor' can be address(0)
emit EggDeposited(depositor, tokenId);
}

The corresponding withdrawEgg function requires that msg.sender matches the stored eggDepositors[tokenId] to allow withdrawal.

// src/EggVault.sol
function withdrawEgg(uint256 tokenId) public {
require(storedEggs[tokenId], "Egg not in vault");
// If eggDepositors[tokenId] is address(0), this check will always fail
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 a user mistakenly calls depositEgg with depositor set to address(0), the eggDepositors[tokenId] mapping will record the zero address for that specific tokenId. Consequently, the require(eggDepositors[tokenId] == msg.sender, "Not the original depositor") check in withdrawEgg will always fail because msg.sender can never be the zero address.

Impact

If an NFT is deposited with the depositor address set to address(0), it becomes permanently locked within the EggVault contract. No user, including the original owner or the contract owner, will be able to withdraw the NFT via the withdrawEgg function, leading to a permanent loss of the asset for the user.

Tools Used

Manual Review

Recommendations

Add a require statement at the beginning of the depositEgg function to ensure the depositor address is not the zero address.

function depositEgg(uint256 tokenId, address depositor) public {
require(depositor != address(0), "Depositor cannot be zero address"); // Add this check
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor;
emit EggDeposited(depositor, tokenId);
}
Updates

Lead Judging Commences

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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