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

Anyone can become owner of `MysteryBox`

BigDog

Description:

Due to a lack of checks in MysteryBox::changeOwner, any user at any time can make themselves the new owner of the protocol.

Impact:

The owner "Can set the price of boxes, add new rewards, and withdraw funds", meaning any user could perform a takeover of the critical functionalities of the protocol. Also, different users could call this function repeatedly, making a game of hot potato with who owns and controls the contract.

Proof of Concept:

function testAnyoneCanBecomeOwner() public {
address contractAddr = address(mysteryBox);
uint256 slot = 0;
bytes32 storedValue = vm.load(contractAddr, bytes32(slot));
address storedOwner = address(uint160(uint256(storedValue)));
console.log(storedOwner); // mysteryBox owner at deployment (msg.sender)
​
vm.prank(user1);
mysteryBox.changeOwner(user1);
assertEq(mysteryBox.owner(), user1);
}

While MysteryBox was initially owned by the deployer of the contract, a random address (user1) was able to change ownership of the contract over to themselves.

Recommended Mitigation:

Make use of OpenZeppelin's Ownable standards, or add in the makeshift check used throughout the code:

function changeOwner(address _newOwner) public {
+ require(msg.sender == owner, "Only owner transfer ownership");
owner = _newOwner;
}
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!