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

Unrestricted access to depositEgg function allows NFT theft

teoslaf

Summary

The EggVault::depositEgg function allows any address to claim ownership of deposited NFTs by setting themselves as the depositor, enabling theft of NFTs that have been transferred to the vault.

Vulnerability Details

The depositEgg function is public and accepts an arbitrary depositor address without any access control or verification:

## EggVault.sol
​
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);
}

The function only checks that:

  1. The NFT is owned by the vault

  2. The NFT hasn't already been deposited

It does not verify who is calling the function or that the depositor parameter is legitimate. This allows anyone to claim ownership of an NFT that has been transferred to the vault but not yet registered in the storedEggs mapping.

Proof of Concept

  1. Alice calls EggHuntGame::depositEggToVault(tokenId) to deposit her NFT

  2. The function transfers the NFT to the vault and then calls eggVault.depositEgg(tokenId, msg.sender)

  3. Mallory monitors the mempool and sees Alice's transaction

  4. Mallory front-runs Alice's transaction with a direct call to eggVault.depositEgg(tokenId, mallory)

  5. Mallory's transaction completes first, registering Mallory as the depositor

  6. Alice's transaction fails with "Egg already deposited" error

  7. Mallory can now call withdrawEgg(tokenId) to steal Alice's NFT

Impact

  • Allows theft of NFTs that have been transferred to the vault

  • Undermines the core security model of the vault system

  • Creates a race condition for depositing NFTs

  • Users could lose valuable assets

Tools Used

  • Manual code review

Recommendations

Implement proper access control on the depositEgg function:

+ address public gameContract;
+
+ modifier onlyGame() {
+ require(msg.sender == gameContract, "Caller is not the game contract");
+ _;
+ }
+
+ function setGameContract(address _gameContract) external onlyOwner {
+ require(_gameContract != address(0), "Invalid game contract address");
+ gameContract = _gameContract;
+ }
​
- 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 7 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.