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

There is no ownership check on EggVault::depositEgg function, anyone can call the function

markomarko

Summary

The EggVault::depositEgg function has no checking system for a depositor, allowing anyone to call this function without ensuring that the depositor is the rightful owner of the tokenId.

Vulnerability Details

Proof of Concept:

  1. Mint egg to vault

  2. Attacker calls EggVault::depositEgg and the function will think that this is a valid depositor

  3. Attacker calls EggVault::withdrawEgg, freely withdraw his stolen NFT from the vault.

Proof of Code:

Add the following code to the EggHuntGameTest.t.sol file.

function test_Attacker_Steals_Egg_FromVault() public {
vm.prank(address(game));
nft.mintEgg(address(vault), 999);
​
// assuming bob is the attacker
vm.prank(bob);
vault.depositEgg(999, bob);
​
assertEq(vault.eggDepositors(999), bob);
assertTrue(vault.isEggDeposited(999));
​
vm.prank(bob);
vault.withdrawEgg(999);
​
assertEq(nft.ownerOf(999), bob);
}

Impact

  1. Attackers can withdraw other people's NFTs

  2. Original depositor cannot withdraw his NFT

Tools Used

  1. Foundry

Recommendations

To prevent this problem, we should add an NFT ownership check before assigning a depositor.

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

Lead Judging Commences

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

Frontrunning Vulnerability DepositEgg

Front-running depositEgg allows deposit ownership hijacking.

Appeal created

farismaulana Auditor
5 months ago
m3dython Lead Judge
5 months ago
farismaulana Auditor
5 months ago
m3dython Lead Judge
5 months ago
m3dython Lead Judge 5 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.