Eggstravaganza

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

Deploying an `EggHuntGame` using an `EggstravaganzaNFT` that has a different `gameContract` set and/or using an `EggVault` that has a different `EggstravaganzaNFT` set will not allow its users to mint and/or deposit their found egg NFTs.

Description:
EggHuntGame can be deployed with an EggstravaganzaNFT address, which has a gameContract set to either address(0) or a different gameContract. Until the EggstravaganzaNFT owner sets the newly deployed EggHuntGame contract through EggstravaganzaNFT::setGameContract, users won't be able to mint any found eggs.
EggHuntGame can be deployed with an EggVault address, which has a eggNFT set to either address(0) or a different EggstravaganzaNFT than the one used in the game. Until the EggVault owner sets same eggNFT contract through EggVault::setEggNFT, users won't be able to deposit any minted eggs.

Impact:
Users won't be able to mint and/or deposit any found eggs.

Proof of Concept:

I. User can't mint an egg.

  1. Owner #1 deploys EggstravaganzaNFT with no gameContract address.

  2. Owner #2 deploys a EggHuntGame using the EggstravaganzaNFT deployed by owner #1.

  3. Owner #2 starts a new game.

  4. User wins an egg, but can't mint it.

II. User can't deposit an egg.

  1. Owner #1 deploys EggVault with no eggNFT address.

  2. Owner #2 deploys a EggHuntGame using the EggVault deployed by owner #1.

  3. Owner #2 starts a new game.

  4. User mints an egg, but can't deposit it.

Proof of Code:

I. User can't mint an egg.

Comment out the following line in EggGameTest::setUp function.

function setUp() public {
...
// Set the allowed game contract for minting eggs in the NFT.
- nft.setGameContract(address(game));
+ // nft.setGameContract(address(game));
...
}

and place the following test in the same contract

function testCantMintNFTIfGameNotSet() public {
vm.prank(address(game));
vm.expectRevert("Unauthorized minter");
nft.mintEgg(alice, 20);
}

II. User can't deposit an egg.

Comment out the following line in EggGameTest::setUp function.

function setUp() public {
...
// Set the allowed game contract for minting eggs in the NFT.
- vault.setEggNFT(address(nft));
+ // vault.setEggNFT(address(nft));
...
}

and place the following test in the same contract

function testCantMintNFTIfNFTNotSetInVault() public {
vm.prank(address(game));
nft.mintEgg(alice, 20);
assertEq(nft.ownerOf(20), alice);
vm.prank(alice);
nft.approve(address(game), 20);
vm.prank(alice);
vm.expectRevert();
game.depositEggToVault(20);
}

Recommended Mitigation:

  1. Before starting a game have checks inside the deployed EggHuntGame that EggstravaganzaNFT::gameContract is address(this) and EggVault::eggNFT is EggHuntGame(address(this))::eggNFT so for every started game contracts will match.

  2. Use the suggestion from #1, but implement the EggVault::eggNFT check inside the contructor of EggHuntGame so the game can be deployed only if the contracts match, removing one check on game start and saving gas on contract deployment.

Updates

Lead Judging Commences

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

Support

FAQs

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