Submission Details
Severity:
`MysteryBox::changeOwner` has no access control, let everyone can change the owner!
Summary
In changeOwner function, it only do one thing : change the owner, and the owner has all the setting power of setBoxPrice, addReward and withdrawFunds. So the owner has power to drain the protocol.
Vulnerability Details
user2 call
changeOwnerto himself.now the owner is user2!!!
Add following code to test file:
function testAnyOneCanChangeOwner() public {
assertEq(mysteryBox.owner(), owner);
vm.prank(user2);
mysteryBox.changeOwner(user2);
assertEq(mysteryBox.owner(), user2);
}
Impact
Malicious user can change the owner to themselves, then maybe set the very expecsive value rewards, and even more they can take away all money of protocol by calling withdrawFunds
Tools Used
manual review
Recommendations
Add access control!
function changeOwner(address _newOwner) public {
+ require(msg.sender == owner, "Only owner can change owner");
owner = _newOwner;
}
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.