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

`EggstravaganzaNFT` owner can set a new game contract through `setGameContract` not allowing both old and new `EggHuntGame` users to mint their found egg NFTs.

skaltso

Description:
EggstravaganzaNFT owner can set a new gameContract through EggstravaganzaNFT::setGameContract. The old gameContract will no longer be able to mint NFTs due to the check in EggstravaganzaNFT::mintEgg that checks whether the minter is the newly set gameContract.

function mintEgg(address to, uint256 tokenId) external returns (bool) {
require(msg.sender == gameContract, "Unauthorized minter");
...
}

If the old gameContract has minted more NFTs than the new one (its tokenId is bigger), then the new gameContract won't be able to mint new NFTs because the supplied tokenId would already have been minted.

function mintEgg(address to, uint256 tokenId) external returns (bool) {
...
_mint(to, tokenId);
...
}

Impact:
Users that have successfully found an egg with both the old and the new EggHuntGame contract cannot mint it, thus not providing them with their rightful acquisition.

Proof of Concept:

  1. User gets an egg from the game.

  2. The nft owner changes the gameContract address to a new one with no minted eggs.

  3. Users from both games that win eggs cannot mint them.

Proof of Code:
Create a new EggHuntGame contract and set it up in EggHuntGameTest.t.sol and place the following test in the same file.

function testCantMintEggIfNFTChangesGameAddress() public {
testDepositEggToVault();
​
vm.prank(address(owner));
nft.setGameContract(address(newGame));
​
vm.prank(address(game));
vm.expectRevert("Unauthorized minter");
nft.mintEgg(bob, 21);
​
vm.prank(address(newGame));
vm.expectRevert(abi.encodeWithSignature("ERC721InvalidSender(address)", address(0)));
nft.mintEgg(bob, 20);
}

Recommended Mitigation:
Have a mapping of allowed EggHuntGames that can mint NFTs in the EggstravaganzaNFT::mintEgg function and use its totalSupply to keep track of tokenIds instead of EggHuntGame::eggCounter variable.

mapping(address => bool) public gameContracts;
​
function addGameContract(address _gameContract) external onlyOwner {
require(_gameContract != address(0), "Invalid game contract address");
gameContracts[_gameContract] = true;
}
​
function mintEgg(address to) external returns (bool) {
require(gameContracts[msg.sender], "Unauthorized minter");
_mint(to, totalSupply);
totalSupply += 1;
return true;
}

Or use openzeppelin's access control for authorization.

Updates

Lead Judging Commences

m3dython Lead Judge 4 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.