Mystery Box

Summary

The changeOwner(address _newOwner) function in the MysteryBox contract
currently allows any user to change the contract's owner to an arbitrary
address. This lack of access control creates a significant security
vulnerability, enabling any external user to call this function and assign
ownership to themselves or another address.

Vulnerability Details

In the MysteryBox::changeOwner() function, there are no access control
checks implemented, which permits any user to change the contract's
ownership. This is a critical vulnerability as ownership typically grants
significant control over the contract, including the ability to perform
privileged actions such as withdrawing funds or managing key contract
parameters.

Impact

An attacker could exploit this vulnerability by calling the changeOwner()
function to set themselves or another address as the new owner. With
control over the contract's ownership, the attacker could:

1. Modify the Box Price: Call setBoxPrice() to change the price of mystery
   boxes to an arbitrary value, potentially leading to financial loss for
   legitimate users.

2. Manipulate Rewards: Call addReward() to alter the rewards system,
   undermining the contract's intended functionality.

3. Drain Funds: Call withdrawFunds() to extract the contract's ETH balance,
   resulting in the loss of funds for all users.

Tools Used

Manual Code Review

Recommendations

To mitigate this vulnerability, it is essential to restrict the
changeOwner() function so that only the current owner of the contract can
invoke it. This can be achieved by implementing an access control
modifier, such as onlyOwner.

Implementation Steps

Add the onlyOwner Modifier: Define the onlyOwner modifier to check that
the caller is the current owner of the contract:

Modify the changeOwner() function to include the onlyOwner modifier,
ensuring that only the current owner can change ownership.

Secure Other Sensitive Functions: Additionally, apply the onlyOwner
modifier to other sensitive functions like setBoxPrice(), addReward(),
and withdrawFunds(). This approach not only enhances security but also
saves gas costs compared to using multiple require statements:

By implementing these recommendations, the MysteryBox contract will be
significantly more secure, protecting it from unauthorized ownership
changes and potential exploitation.