Eggstravaganza

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

Calling EggVault:setEggNFT() will corrupt the entire game state

Description

As the name suggests this function allows the vault owner to change the address of the EggstravaganzaNFT'contracts:

function setEggNFT(address _eggNFTAddress) external onlyOwner {
require(_eggNFTAddress != address(0), "Invalid NFT address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
}

The issue lies in the fact that the vault's global variables storedEggs and eggDepositors do not account for such change, they will be always relative to the NFT contract before the change.

Impact

  • searchForEgg() and depositEggToVault() becomes useless since they will always refer to the old NFT contract

  • EggVault:depositEgg() will not work for tokenIds already deposited when the old NFT contract was active

Recomendations

To correctly support such functionality the EggVault's tracking variables should be updated per-NFT-contract:

-mapping(uint256 => bool) public storedEggs;
+mapping(EggstravaganzaNFT => mapping(uint256 => bool)) public storedEggs;
-mapping(uint256 => address) public eggDepositors;
+mapping(EggstravaganzaNFT => mapping(uint256 => address)) public eggDepositors;

Also, the EggHuntGame contract shoule be updated to fetch the currently active eggNFT from the EggVault to avoid discrepancies.

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.