Mystery Box

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

No access control in the changeOwner function

Summary

The function changeOwnercan be called by anyone.

Vulnerability Details

On line 111 of the code, the changeOwner function does not implement proper access control, allowing any user to set the owner of the contract to any arbitrary address.

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

Impact

An attacker can call the changeOwner function to take ownership of the contract. Once ownership is gained, the attacker can set the box price, add rewards, and worst of all, call the withdrawFunds function to steal all contract funds.

Tools Used

Manual inspection.

Recommendations

Implement access control to restrict the changeOwner function to the current owner only. This can be done by adding a require statement:

function changeOwner(address _newOwner) public {
require(msg.sender == owner, "Only owner can change owner");
owner = _newOwner;
}

Alternatively, use a modifier and a custom error for better readability and gas optimization:

error NotOwner();
modifier onlyOwner {
if (msg.sender != owner) {
revert NotOwner();
}
_;
}
function changeOwner(address _newOwner) external onlyOwner {
owner = _newOwner;
}

Apply the onlyOwner modifier to other functions that require ownership control. Additionally, since the function is called externally, it should be marked as external instead of public.

Updates

Appeal created

inallhonesty Lead Judge 9 months 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.