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

Missing Access Control on `depositEgg` Allows Unauthorized Depositor Assignment

SashaFlores

Summary

The depositEgg function in the EggVault contract lacks proper access control, allowing any external caller to register an egg as deposited and assign an arbitrary depositor address. This could result in inconsistencies in game logic and misattribution of ownership.

Vulnerability Details

The depositEgg function is marked public and can be called by anyone. While the contract checks that the NFT is already transferred to the vault and that it hasn't been deposited before, it does not restrict who can perform the call.

This means that a malicious actor could call the function before the legitimate game contract does, assigning themselves or another incorrect address as the depositor of a given token ID.

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

Proof of Concept

  1. Attacker waits for a user to transfer an egg NFT to the vault via depositEggToVault in EggHuntGame.

  2. Attacker front-runs the expected call to depositEgg by calling below.

  3. The egg is now recorded as deposited by the attacker instead of the actual user, which can impact logic dependent on eggDepositors.

eggVault.depositEgg(tokenId, attackerAddress);

Impact

  • Incorrect attribution of egg ownership within the vault.

  • Potential to disrupt reward systems, eligibility checks, or future logic based on the depositor field.

  • No direct loss of funds or NFTs due to existence of ownerOf and duplicate guard checks.

This is a Low severity issue per CodeHawks standards, as it introduces a logic flaw and potential for misbehavior but does not endanger assets directly or indirectly.

Recommendations

Restrict the depositEgg function so it can only be called by the game contract:

address public gameContract;
​
modifier onlyGameContract() {
require(msg.sender == gameContract, "Only game contract can call this");
_;
}
​
function depositEgg(uint256 tokenId, address depositor) public 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 5 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.