Submission Details
Severity:
No access controls on `MysteryBox.sol::changeOwner` can lead to complete loss of funds
Summary
The changeOwner function, as it's name implies, changes the owner address for a given address, but the function itself lacks access control, meaning anyone can call the function and set himself as owner, giving him access to the withdrawFunds function, allowing him to steal all the funds in the contract.
Vulnerability Details
You may add the following PoC to TestMysteryBox.t.sol to corroborate the issue
function testUserCanChangeOwnerAndDrainFunds() public {
//Assert that address owner is the owner of the contract.
assertEq(mysteryBox.owner(), owner);
//User1 calls changeOwner and sets himself as owner
vm.prank(user1);
mysteryBox.changeOwner(user1);
assertEq(mysteryBox.owner(), user1);
//User1 can now withdraw all the funds in the contract
vm.prank(user1);
mysteryBox.withdrawFunds();
}
Impact
Anyone can set himself as owner and drain the contract's ether.
Tools Used
manual review
Recommendations
Add a onlyOwner requirement to changeOwner
Updates
Appeal created
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
Anyone can change owner
Rewards breakdown
nSLOC
86This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.