Mystery Box

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

[H-1] Unrestricted changeOwner Function (Insecure Access Control + Ownership Privilege Abuse)

Description: The smart contract mysterybox contains a function named changeOwner that allows any user to modify the contract's ownership. This lack of proper access control allows unauthorized users to assume ownership of the contract, giving them full control over its operations. Once a malicious actor gains ownership, they can potentially execute privileged functions, including withdrawing all funds using withdrawFunds function or changing core logic, leading to significant financial and operational damage.

Impact:

  1. Unauthorized Access: Any user can become the contract owner without restriction.

  2. Fund Theft: The new unauthorized owner can withdraw all funds from the contract.

  3. Contract Control: The malicious owner can execute privileged actions, modify the contract.

Proof of Concept:
paste this code snipped inside TestMysteryBox.t.sol and run this command in the terminal forge test --mt testOwnerChange -vvvv to see the full output of this test

function testOwnerChange() public{
// 2 users buys box
vm.deal(user1,5 ether);
vm.prank(user1);
mysteryBox.buyBox{value:0.1 ether}();
vm.deal(user2,2 ether);
vm.prank(user2);
mysteryBox.buyBox{value:0.1 ether}();
//One of the users calls the change owner function and becomes the owner
vm.prank(user1);
mysteryBox.changeOwner(user1);
assertEq(mysteryBox.owner(), user1);
//The current owner withdraws all the funds
uint256 ownerBalanceBefore = user1.balance;
console.log("Owner Balance Before:", ownerBalanceBefore);
vm.prank(user1);
mysteryBox.withdrawFunds();
uint256 ownerBalanceAfter = user1.balance;
console.log("Owner Balance After:", ownerBalanceAfter);
}

Recommended Mitigation:

  1. Implement strict access control on the changeOwner function by adding a require statement to ensure that only the current owner can call this function.

function changeOwner(address _newOwner) public {
+ require(msg.sender == owner);
owner = _newOwner;
}
  1. Consider using OpenZeppelin’s Ownable contract, which provides a secure ownership mechanism.

Tools Used: Manual

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!