Eggstravaganza

First Flight #37
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: low
Invalid

`onlyOwner` can mint themselves the `eggNFT`

Summary

In EggstravaganzaNFT.sol the onlyOwner can set their address as the gameContract. Which is then used to mint eggNFT, and if approved the onlyOwner can then mint themselves the eggNFT

Vulnerability Details

/// @notice Only the owner can set the game contract allowed to mint eggs.
function setGameContract(address _gameContract) external onlyOwner {
require(_gameContract != address(0), "Invalid game contract address");
gameContract = _gameContract;
}
/// @notice Public function to mint a new Eggstravaganza NFT.
/// Only the approved game contract can mint eggs.
function mintEgg(address to, uint256 tokenId) external returns (bool) {
require(msg.sender == gameContract, "Unauthorized minter");
_mint(to, tokenId);
totalSupply += 1;
return true;
}

In EggstravaganzaNFT.sol in the setGameContract function it takes a parameter of _gameContract and of type address. Which then is set to gameContract. Then it's used in the mintEgg function as a require check, which if passed then you can mint the eggNFT to the address that is passed in the parameter. The problem is that the address
that is minted to should be the gameContract and the address of the gameContract should be the EggHuntGame.sol, this is all inputted by the onlyOwner. The problem is onlyOwner is a trusted user, but they could act in bad faith and set the address of the gameContract to their own. And then in mintEgg function they can set their wallet address in the parameter. This will allow them to mint themselves the eggNFT. This can happen because in the mintEgg function there's a require statement that states require(msg.sender == gameContract, "Unauthorized minter");. This passes since the onlyOwner set the gameContract address to their own and msg.sender has to be the address of the gameContract, which the address of msg.sender is the address of onlyOwner since onlyOwner is the one that called the function.

Impact

onlyOwner can mint themselves the eggNFT and take it all if they decided to act in bad faith.

Tools Used

Manual Review

Recommendations

Use this code instead

// Role definition
bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
// Hardcoded allowed minter contract address
address public constant ALLOWED_MINTER_CONTRACT = 0xAbC123...; // Replace with actual EggHuntGame.sol contract address
// Stored game contract address
address public gameContract;
/// @notice Grants the MINTER_ROLE to the allowed game contract.
/// Only the owner can call this.
function grantMinter(address _addr) external onlyOwner {
require(_addr != msg.sender, "Cannot assign minter role to yourself");
require(_addr == ALLOWED_MINTER_CONTRACT, "Only the approved contract can be granted minter role");
_grantRole(MINTER_ROLE, _addr);
}
/// @notice Sets the game contract address.
/// Only the allowed game contract can be set.
function setGameContract(address _gameContract) external onlyOwner {
require(_gameContract == ALLOWED_MINTER_CONTRACT, "Only the approved contract can be set");
gameContract = _gameContract;
}
/// @notice function to mint a new Eggstravaganza NFT. Which will then be minted to the game contract.
/// Can only be called by the minter contract.
function mintEgg(uint256 tokenId) external onlyRole(MINTER_ROLE) returns (bool) {
require(msg.sender == gameContract, "Unauthorized minter");
_mint(msg.sender, tokenId);
totalSupply += 1;
return true;
}

This prevents unauthorized address even the contract owner from minting and only allows the game contract to mint the NFT.

Updates

Lead Judging Commences

m3dython Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Trusted Owner

Owner is trusted and is not expected to interact in ways that would compromise security

Appeal created

juanalacubana Submitter
5 months ago
m3dython Lead Judge
5 months ago
m3dython Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Trusted Owner

Owner is trusted and is not expected to interact in ways that would compromise security

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.