Submission Details
Severity:
Missing access control for the `changeOwner` function making possible for anyone to change the `MysteryBox` contract ownership
Summary
The MysteryBox::changeOwner function is missing access control.
Vulnerability Details
The MysteryBox::changeOwner function doesn't check if the caller (msg.sender) is the owner of the contract before assigning the specified address of the _newOwner parameter to the owner variable.
Proof of Concept
Add this test to the test suite:
function test_changeOwnerIsMissingAccessControl() public {
// Check if `owner` is the owner of the MysteryBox contract
assertEq(mysteryBox.owner(), owner);
// Impersonates a non-owner address `user1`
vm.startPrank(user1);
// Give the ownership of the MysteryBox contract to `user2`
mysteryBox.changeOwner(user2);
// Stops impersonating `user1`
vm.stopPrank();
// Check if `user2` is the new owner of the MysteryBox contract
assertEq(mysteryBox.owner(), user2);
}
And execute it:
forge test --match-test test_changeOwnerIsMissingAccessControl
Impact
Anyone can change the owner of the contract to an arbitrary address.
Tools Used
Manual review
GNU Emacs (solidity-mode)
Foundry tests
Recommendations
Modify the MysteryBox::changeOwner function, checking if the caller is the MysteryBox contract owner:
modified src/MysteryBox.sol
contract MysteryBox {
}
function changeOwner(address _newOwner) public {
+ require(msg.sender == owner, "Only the owner can change the contract ownership");
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.