Mystery Box

First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Unrestricted Change of Ownership Leading to Potential Contract Hijacking

Vulnerability Details

The MysteryBox::changeOwner lacks critical security checks, including zero address validation for the new owner, access control restrictions, and event emissions to log ownership changes. These omissions can lead to accidental or malicious transfer of contract ownership, potentially resulting in loss of control over the contract's critical functions.

POC

function changeOwner(address _newOwner) public {
owner = _newOwner;
}

In this example, anyone can call changeOwner() and set the owner to any address, including the zero address.

Impact

  1. Potential loss of contract control

  2. Irreversible transfer to invalid addresses

  3. Lack of transparency in ownership changes

  4. Unauthorized transfers by any user

Tools Used

Manual Review, Foundry

Recommendations

Implement zero address validation:

+ require(newOwner != address(0), "New owner cannot be zero address");

Add access control:

+ require(msg.sender == owner, "Only current owner can transfer ownership");

Emit an event for ownership transfer:

+ event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

Implement change of ownership process:

+ function changeOwner(address newOwner) public {
+ require(msg.sender == owner, "Only current owner can initiate transfer");
+ require(newOwner != address(0), "New owner cannot be zero address");
+ pendingOwner = newOwner;
+ }

These changes will significantly enhance the security and transparency of the ownership transfer process.

Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Anyone can change owner

Support

FAQs

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

Give us feedback!