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

NFT Contract Mismatch Preventing Withdrawals

eightysix

Summary

A vulnerability exists in the Eggstravaganza vault system that prevents users from withdrawing tokens they have previously deposited when the NFT contract reference is updated. After the contract update, tokens from the old NFT contract are no longer recognized by the vault, causing failed withdrawal attempts for users who deposited tokens before the update.

Vulnerability Details

The vulnerability arises when the Egg Vault's NFT contract reference is updated using the setEggNFT() function. If a user has previously deposited an NFT token under the old contract, they are unable to withdraw that token after the contract reference is changed. This occurs because the vault attempts to check ownership using the new NFT contract, which does not recognize the old contract’s tokens, leading to failed withdrawals.

Impact

  • Failed withdrawals: Users who have deposited tokens from the old NFT contract cannot withdraw those tokens after the contract update.

  • Loss of funds: Deposited tokens remain locked in the vault, and users may mistakenly believe their tokens are lost or inaccessible.

  • User frustration: Players will experience failed withdrawal transactions, leading to confusion and frustration.

  • Vault mismanagement: The vault may hold tokens that are no longer retrievable due to the mismatch between contracts.

POC

function testWithdrawFailsWithMismatchedNFTContract() public {
// Mint an egg to Alice via the original game/NFT
vm.prank(address(game));
nft.mintEgg(alice, 20);
assertEq(nft.ownerOf(20), alice);
​
// Alice deposits the egg into the vault
vm.prank(alice);
nft.approve(address(game), 20); // Alice approves the transfer for token 20
vm.prank(alice);
game.depositEggToVault(20); // Alice deposits token 20 into the vault
​
// Owner changes the Vault's NFT contract to a new one
vm.startPrank(owner);
EggstravaganzaNFT nft2 = new EggstravaganzaNFT("Eggstravaganza", "EGG2");
vault.setEggNFT(address(nft2)); // Vault now points to nft2
vm.stopPrank();
​
// Alice tries to withdraw the token (from the original NFT contract)
// Vault is now pointing to nft2, and token 20 is not recognized
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, 20));
vm.prank(alice);
vault.withdrawEgg(20); // Withdrawal fails due to NFT mismatch
}

In the above Proof of Concept (PoC), the test first ensures that Alice deposits token 20 (minted under the original NFT contract) into the vault. After the contract reference is changed, the vault points to the new NFT contract (nft2), and Alice is unable to withdraw her previously deposited token due to the contract mismatch.

Tools Used

  • Foundry

  • Manual Review

Recommendations

  • Nft contracts should be set only once 'during deployment at the constructor level '

Updates

Lead Judging Commences

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

Support

FAQs

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

Give us feedback!