Mystery Box

Summary

The changeOwner function should have a visibility modifier to restrict access. It should also include a check to ensure only the current owner can change the owner.

Vulnerability Details

No Access Restriction:

The function lacks access control, meaning anyone can call this function and change the contract's owner to any address, which is a huge security vulnerability.
This can result in a loss of control over the contract, allowing malicious actors to take over ownership and perform any privileged actions.

Lack of Ownership Check:

The function does not verify if the caller is the current owner. This means any arbitrary user can change the owner without restriction.

Findings

https://github.com/Cyfrin/2024-09-mystery-box/blob/281a3e35761a171ba134e574473565a1afb56b68/src/MysteryBox.sol#L111C1-L113C6

Impact

Tools Used

Manual analysis

Recommendations

Add a Modifier to Restrict Access:

You need to add a modifier (e.g., onlyOwner) that ensures only the current owner can call this function.

Add a Check to Ensure Ownership:

Use the require statement to ensure that only the current owner can change the owner.