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

No Approval Revocation When Withdrawing NFTs from Vault

mishoko

Summary

When an NFT is withdrawn from the EggVault, the contract doesn't revoke any existing approvals that might have been set while the NFT was in the vault's custody. This oversight could allow previously approved operators to maintain access to the NFT even after it has been returned to its original owner.

Vulnerability Details

n the withdrawEgg function of the EggVault contract, NFTs are transferred back to the original depositor, but any prior approvals set on the NFT are not cleared:

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 at any point while the NFT is in the vault an address is approved to transfer it (through approve or setApprovalForAll), that approval will remain active after the NFT is withdrawn. This could occur through:

  1. Admin functions that approve marketplace contracts to handle NFTs

  2. Compromised admin accounts that set malicious approvals

  3. Legitimate integrations that require approval access during vault custody

Following best security practices, approvals should be cleared when ownership changes, especially when returning NFTs from a custodial contract.

Impact

The impact of this vulnerability is low to medium, as it requires specific conditions:

  1. An approval must be set while the NFT is in the vault

  2. The approved address must be malicious or compromised

  3. The user must not check or revoke approvals manually after withdrawal

However, if these conditions are met, the consequences could be:

  • An approved address could transfer the NFT away from the user without their consent

  • Users may not realize that their NFTs have lingering approvals after withdrawal

Tools Used

  • Manual code review

  • Foundry for creating proof of concept test

Recommendations

Modify the withdrawEgg function to clear approvals before transferring the NFT back to the user:

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];
​
// Clear any approvals before transferring
eggNFT.approve(address(0), tokenId);
eggNFT.transferFrom(address(this), msg.sender, tokenId);
emit EggWithdrawn(msg.sender, tokenId);
}

This simple addition ensures that the NFT is returned to the user without any lingering approvals, providing a cleaner security model and protecting users from unexpected behavior.

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.