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

Insufficient Access Control in EggVault.depositEgg

zhouhuan19910202

Summary

The EggVault contract’s depositEgg function lacks proper access control, allowing anyone to call it and assign ownership of an NFT to an arbitrary address after transferring it to the Vault. This vulnerability enables attackers to steal ownership of deposited NFTs, bypassing the intended game mechanics.

Vulnerability Details

The depositEgg function in EggVault is defined as public without caller restrictions:

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

Key issues:

  • No Caller Restriction: Any address can call depositEgg and set depositor to themselves or another address, as long as the NFT is already in the Vault.

  • Interaction with EggHuntGame: While EggHuntGame.depositEggToVault calls this function correctly, an attacker can bypass it by:

    1. Transferring an NFT to the Vault via eggNFT.transferFrom.

    2. Calling depositEgg directly with their own address as depositor.

An attacker can:

  • Transfer an NFT to the Vault independently.

  • Register it under their address, effectively claiming ownership.

  • Later call withdrawEgg to retrieve it.

Impact

This vulnerability disrupts the system:

  1. NFT Theft: Attackers can claim ownership of NFTs deposited by others, stealing assets.

  2. Game Integrity: Bypasses the intended deposit flow through EggHuntGame, breaking gameplay rules.

  3. Economic Loss: Stolen NFTs reduce trust and value in the ecosystem.

  4. Player Frustration: Legitimate players lose control over their assets, potentially abandoning the game.

Tools Used

  • Manual code review

  • Solidity compiler analysis (version ^0.8.23)

  • Scenario testing with hypothetical attack contracts

Recommendations

  1. Restrict Caller Access:

    • Limit depositEgg to only be callable by the EggHuntGame contract.

  2. Validate Depositor:

    • Ensure depositor matches the actual sender or NFT owner at transfer time (though current design relies on EggHuntGame).

Example Implementation

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.23;
​
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "./EggstravaganzaNFT.sol";
​
contract EggVault is Ownable, ReentrancyGuard {
EggstravaganzaNFT public eggNFT;
address public gameContract; // Authorized caller
​
mapping(uint256 => bool) public storedEggs;
mapping(uint256 => address) public eggDepositors;
​
event EggDeposited(address indexed depositor, uint256 tokenId);
event EggWithdrawn(address indexed withdrawer, uint256 tokenId);
​
constructor(address _eggNFTAddress, address _gameContract) Ownable(msg.sender) {
require(_eggNFTAddress != address(0), "Invalid NFT address");
require(_gameContract != address(0), "Invalid game contract address");
eggNFT = EggstravaganzaNFT(_eggNFTAddress);
gameContract = _gameContract;
}
​
function setGameContract(address _gameContract) external onlyOwner {
require(_gameContract != address(0), "Invalid game contract address");
gameContract = _gameContract;
}
​
function depositEgg(uint256 tokenId, address depositor) public {
require(msg.sender == gameContract, "Only game contract can deposit");
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);
}
​
// ... other functions ...
}
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.