Submission Details
Severity:
changeOwner lacks proper access control and anyone can change owner even to zero address
Summary
Vulnerability Details
This function allows anyone to change owner and any user can be owner and withdraw funds which should not happen. even one can cahnge the owner to zero address which is horrible and should be avoided.
Compiler run successful!
Ran 2 tests for test/TestMysteryBox.t.sol:MysteryBoxTest
[PASS] testChangeOwner() (gas: 12889)
Traces:
[12889] MysteryBoxTest::testChangeOwner()
├─ [0] VM::expectRevert(Only owner change ownership to new owner)
│ └─ ← [Return]
├─ [2595] MysteryBox::changeOwner(0x0000000000000000000000000000000000000001)
│ └─ ← [Revert] revert: Only owner change ownership to new owner
└─ ← [Stop]
[PASS] testChangeOwner_AccessControl() (gas: 19416)
Traces:
[19416] MysteryBoxTest::testChangeOwner_AccessControl()
├─ [0] VM::prank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [5533] MysteryBox::changeOwner(0x0000000000000000000000000000000000000001)
│ └─ ← [Stop]
├─ [404] MysteryBox::owner() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000001
├─ [0] VM::assertEq(0x0000000000000000000000000000000000000001, 0x0000000000000000000000000000000000000001) [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 1.30ms (333.78µs CPU time)
function testChangeOwner() public {
vm.expectRevert("Only owner change ownership to new owner");
mysteryBox.changeOwner(user1);
}
function testChangeOwner_AccessControl() public {
vm.prank(owner);
mysteryBox.changeOwner(user1);
assertEq(mysteryBox.owner(), user1);
}
Impact
any user will be able to make himself owner and be able to withdraw funds to set the price for the box.
Tools Used
it should be checked that only owner can assign the ownership to someone else and that new owner should not be a zero address.
function changeOwner(address _newOwner) public {
+ require(msg.sender == owner, "Only owner change ownership to new owner");
+ require(_newOwner != address(0), "Invalid address");
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.