Eggstravaganza

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

Contracts inherit from `Ownable` instead of `Ownable2Step`

Summary

The EggHuntGame, EggVault, and EggstravaganzaNFT contracts inherit from OpenZeppelin's Ownable contract, which uses a single-step ownership transfer process. This can lead to accidental loss of ownership if the owner transfers it to the wrong address.

Vulnerability Details

OpenZeppelin's standard Ownable contract (@openzeppelin/contracts/access/Ownable.sol) implements a transferOwnership function that changes the owner in a single transaction. If the newOwner address provided is incorrect (e.g., a typo, wrong address copied, or an address that cannot manage the contract like address(0) or a non-existent account), the ownership is transferred irrevocably, and administrative control over the contract is lost.

This affects the following contracts:

  • EggHuntGame.sol: Inherits Ownable directly.

    contract EggHuntGame is Ownable { ... }
  • EggVault.sol: Inherits Ownable directly.

    contract EggVault is Ownable { ... }
  • EggstravaganzaNFT.sol: Inherits Ownable along with ERC721.

    contract EggstravaganzaNFT is ERC721, Ownable { ... }

The Ownable2Step variant introduces a safer two-step process (transferOwnership followed by acceptOwnership) where the proposed new owner must actively accept the ownership before the transfer is complete. This prevents locking the contract due to errors in the destination address during the transfer.

Impact

If the owner accidentally transfers ownership to an incorrect or uncontrolled address, all owner-restricted functions in the respective contracts (startGame, endGame, setEggFindThreshold in EggHuntGame; setEggNFT in EggVault; setGameContract in EggstravaganzaNFT) will become unusable. This leads to a permanent loss of administrative control over the contracts.

Tools Used

Manual Review

Recommendations

It is recommended to use the Ownable2Step pattern for all three contracts to mitigate the risk of irreversible ownership loss due to user error.

  1. Import Ownable2Step from @openzeppelin/contracts/access/Ownable2Step.sol.

  2. Change the inheritance from Ownable to Ownable2Step in each contract definition:

    • contract EggHuntGame is Ownable2Step { ... }

    • contract EggVault is Ownable2Step { ... }

    • contract EggstravaganzaNFT is ERC721, Ownable2Step { ... }

  3. Inform the owner/administrator about the new two-step process: transferOwnership(newOwner) must be called first, followed by acceptOwnership() called from the newOwner address.

Updates

Lead Judging Commences

m3dython Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Trusted Owner

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

Single Stepe Owner Transfer

Support

FAQs

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