Mystery Box

Summary

The changeOwner() function in the MysteryBox contract lacks a check against setting the zero address as the new owner. This oversight could potentially lead to an permanent loss of contract ownership if the zero address is accidentally assigned as the owner.

Vulnerability Details

The changeOwner() function is implemented as follows:

This function directly assigns the _newOwner parameter to the owner state variable without any validation. If _newOwner is the zero address (0x0), the contract will lose its owner permanently, as the zero address cannot initiate transactions to transfer ownership back or perform any owner-specific actions.

Impact

The impact of this vulnerability is considered medium:

1. Permanent Loss of Ownership: If the zero address is set as the owner, the contract becomes effectively ownerless, as no one can call owner-restricted functions.

2. Frozen Admin Functions: Critical admin functions like setBoxPrice(), addReward(), and withdrawFunds() become inaccessible.

3. Contract Rigidity: The contract loses its ability to adapt to future needs or fix potential issues that require owner intervention.

While this vulnerability doesn't directly lead to fund loss, it severely impacts the contract's manageability and could indirectly lead to economic losses if critical functions can't be executed.

Tools Used

This vulnerability was discovered through manual code review. No specific tools were required to identify this issue.

Recommendations

To fix this vulnerability, implement a check against the zero address in the changeOwner() function. Here's a recommended implementation:

This implementation:

1. Ensures that the new owner is not the zero address.

2. Adds a check to ensure only the current owner can change ownership (addressing the previously reported issue as well).

3. Emits an event for transparency (you'll need to define this event).

Additionally, consider using a standard ownership management library like OpenZeppelin's Ownable contract, which includes these safety checks and additional useful features.