Mystery Box

Summary

Any malicious(or not) user can change the owner of the contract creating problems for the contarct

Vulnerability Details

The last function of the contract changes the owner of the contract.The vulnerability here is that that there is no restriction for who can change the owner since there is no access control like a modifier or a requirement.So anyone can call the function and create problems for the contract.
The vulnerability can be shown from the test function below:

Running the test gives the following results:

Logs:
The owner is: 0x0000000000000000000000000000000000000001
The owner is: 0x0000000000000000000000000000000000000002

Impact

The impact of the vulnerability is high since any malicious user can become the owner of the contract thus giving them access to 3 important functions of the contract:
1. function setBoxPrice
2. function addReward
3. (Most dangerous function) function withdrawFunds. Gives the malicious user a way to withdraw everything there is in the contract ,stealing everything other users have put in the contract when buying boxes.

Tools Used

Manual/Foundry

Recommendations

There should be an onlyOwner modifier in the changeOwner function making it possible only for the current owner to select a new trusted one.