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

`MysteryBox::changeOwner` is callable by anyone allowing anyone take ownership of the contract.

cryptekmegatron

Summary

The MysteryBox::changeOwner function is publicly accessible, allowing anyone to take ownership of the contract. Since the contract's critical functions—setting the price of boxes, adding rewards, and withdrawing funds—are intended for the owner only, this vulnerability poses a significant risk.

Vulnerability Details

The changeOwner function lacks access control, enabling unauthorized users to change the contract's ownership.

function changeOwner(address _newOwner) public {
// @audit - there are no access controls here
owner = _newOwner;
}

Impact

Anyone can take ownership of the contract, granting them the ability to modify prices, add new rewards, and withdraw funds.

Tools Used

  • VSCodium

  • Manual review

Recommendations

Implement an access control modifier to the changeOwner function to restrict ownership changes.

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

Additionally, update the test function in TestMysteryBox.t.sol to include access control checks:

function testChangeOwner_AccessControl() public {
vm.prank(user1);
+ vm.expectRevert("Only owner can change contract ownership.");
mysteryBox.changeOwner(user1);
- assertEq(mysteryBox.owner(), user1);
}
​
+function testChangeOwner_AccessControl_Owner() public {
+ vm.prank(owner);
+ mysteryBox.changeOwner(user1);
+ assertEq(mysteryBox.owner(), user1);
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year 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.

Give us feedback!