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

Manual update of `EggstravaganzaNFT::setGameContract` or `EggVault::setEggNFT` may result in lost access to previously deposited NFTs

jfornells

Description:

The EggVault::setEggNFT function allowing the owner to change the associated NFT contract (EggstravaganzaNFT) at any time. Similarly, EggstravaganzaNFT exposes setGameContract() to update the mint-authorized game contract (EggHuntGame).

This introduces a configuration and desynchronization risk:

If the NFT contract is updated incorrectly, previously deposited NFTs may become inaccessible.
If new NFTs are deposited with the new contract and the system reverts to the old one, both groups of NFTs may be locked and unrecoverable.
No events are emitted, and no restrictions exist to prevent misconfiguration.

Impact:

  • Permanent loss of access to deposited NFTs.

  • Inability to withdraw assets associated with outdated configuration.

  • Inconsistent behavior difficult to trace or resolve on-chain.

  • Requires off-chain intervention, violating trustless system principles.

Proof of Concept:

function test_setEggNftUpdateError() public gameStarted aliceHasANft {
vm.startPrank(alice);
nft.approve(address(game), 1);
game.depositEggToVault(1);
vm.stopPrank();
assertEq(nft.ownerOf(1), address(vault));
​
vm.prank(owner);
vault.setEggNFT(address(1)); // Misconfigured reference
​
vm.startPrank(alice);
vm.expectRevert(); // Alice can no longer withdraw
vault.withdrawEgg(1);
vm.stopPrank();
}

Result:

Ran 1 test for test/EggHuntGamesTest2.t.sol:EggGameTest2
[PASS] test_setEggNftUpdatError() (gas: 262651)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 17.23ms (8.22ms CPU time)

##Tools Used
Manual review, Foundry

Recommended Mitigation:

​

EggVault Contract:

-- Immutable reference via constructor --
​
- EggstravaganzaNFT public eggNFT;
+ EggstravaganzaNFT public immutable eggNFT;;
​
- constructor()Ownable(msg.sender){}
+ constructor(address _eggNFTAddress) {
+ require(_eggNFTAddress != address(0), "Invalid NFT address");
+ eggNFT = _eggNFTAddress;
+ }
​
- function setEggNFT(address _eggNFTAddress) external onlyOwner {
- require(_eggNFTAddress != address(0), "Invalid NFT address");
- eggNFT = EggstravaganzaNFT(_eggNFTAddress);
- }
​

EggsEggstravaganzaNFT Contract:

-- Immutable reference via constructor --
​
- address public gameContract;
+ address public immutable gameContract;
​
- constructor(string memory _name, string memory _symbol)
+ constructor(string memory _name, string memory _symbol, address _gameContract)
ERC721(_name, _symbol) Ownable(msg.sender)
{
+ require(_gameContract != address(0), "Invalid Game Contract address");
+ gameContract = _gameContract;
}
​
- function setGameContract(address _gameContract) external onlyOwner {
- require(_gameContract != address(0), "Invalid game contract address");
- gameContract = _gameContract;
- }
-- Use upgradeable proxy architecture --

If the system requires the ability to update logic contracts, use an upgradeable proxy (e.g. UUPS or Transparent Proxy):

  • Safely update logic while maintaining state.

  • Avoid manual desynchronization risks.

  • Centralized and auditable upgrade logic.

Updates

Lead Judging Commences

m3dython Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

State corruption

Changing the NFT contract address doesn't update the storedEggs and eggDepositors mappings

Support

FAQs

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