Eggstravaganza

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

NFT Contract Mismatch Leading to Deposit Failures

Summary

A vulnerability exists in the Eggstravaganza game and vault system where the NFT contract is updated after tokens have been minted. If a user attempts to deposit a token from the old contract, the deposit will fail due to a mismatch between the contract the vault is pointing to and the contract where the token was originally minted.

Vulnerability Details

The vulnerability stems from the EggVault contract’s setEggNFT() function, which allows the contract owner to switch the NFT contract that the vault references. After updating the NFT contract, any existing tokens minted from the old contract are no longer valid for deposits into the vault.

The problem arises when a player approves a token for deposit, but the vault points to a new NFT contract where that token ID does not exist, causing the transaction to fail with a ERC721NonexistentToken revert.

This occurs because the depositEggToVault function checks the token's ownership via the current NFT contract. If the token was minted by the previous contract, the vault will fail to recognize it, even though the token exists on the old contract.

Impact

  • Loss of deposit functionality: Users can no longer deposit tokens minted from the old contract once the vault's NFT contract is switched, effectively breaking the intended deposit mechanism.

  • User frustration: Players attempting to deposit eggs from an old contract after the update will experience failed transactions without clear guidance on why their tokens are rejected.

  • Vault mismanagement: The vault may hold tokens that cannot be deposited properly, creating inconsistencies between the deposited token data and the state of the vault.

POC

function testDepositFailsWithMismatchedNFTContract() public {
// Mint an egg to Alice via the original game/NFT
vm.prank(address(game));
nft.mintEgg(alice, 20);
assertEq(nft.ownerOf(20), alice);
// Owner changes the Vault's NFT contract to a new one
vm.startPrank(owner);
EggstravaganzaNFT nft2 = new EggstravaganzaNFT("Eggstravaganza", "EGG2");
vault.setEggNFT(address(nft2));
vm.stopPrank();
// Alice approves the game to transfer her NFT (from both contracts)
vm.prank(alice);
// Expect revert because nft2 does not own token 20
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, 20));
nft2.approve(address(game), 20); // This should fail because nft2 does not own token 20
// Alice approves the game to transfer her NFT (from the original contract)
vm.prank(alice);
nft.approve(address(game), 20); // This is correct — actual token exists here
// Try to deposit the token (from the original NFT)
// But vault is now pointing to nft2, where token 20 does NOT exist
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, 20));
vm.prank(alice);
game.depositEggToVault(20);
}
to nft2, where token 20 does NOT

In the above Proof of Concept (PoC), the test attempts to deposit token 20 that was minted from the original NFT contract after the vault's NFT reference is changed to a new contract. Since the token doesn't exist in the new contract, it reverts with the ERC721NonexistentToken error.

Tools Used

  • Foundry:
    -Manual Review:

Recommendations

1) The nft address should be immutable and only set during contract deployment

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!