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

Unrestricted Access to changeOwner Function Enables Unauthorized Ownership Transfer

ahamedaathal

Summary

The changeOwner function lacks access control, allowing any user to transfer ownership of the contract to an arbitrary address. This grants unauthorized users administrative privileges to manipulate critical contract functions, including setting box prices, adding rewards, and withdrawing funds.

Vulnerability Details

In the MysteryBox.sol contract, the changeOwner function is defined without any access restrictions:

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

This function can be called by any address, enabling any user to set themselves or another address as the new owner. Once ownership is transferred, the malicious owner can perform all administrative actions, including:

  • Setting box prices arbitrarily.

  • Adding or removing rewards.

  • Withdrawing all funds from the contract.

This undermines the contract's security model, as ownership should be restricted to authorized administrators only.

Impact

An attacker can seize complete control over the contract by simply calling the changeOwner function. This allows them to manipulate the contract's parameters and withdraw all funds, resulting in substantial financial losses and loss of user trust.

PoC

An attacker can execute the following transaction to take ownership:

// Attacker calls changeOwner with their own address
await mysteryBox.changeOwner(attackerAddress, { from: attackerAddress });

After this call, the attacker becomes the owner and can execute administrative functions:

// Attacker withdraws all funds
await mysteryBox.withdrawFunds({ from: attackerAddress });

This test verifies that any user, not just the owner, can call the changeOwner function to transfer ownership of the contract. This lack of access control poses a significant security risk.

function testChangeOwnerByNonOwner() public {
// Arrange
address newOwner = user1;
address unauthorizedUser = user2; // This user is not the current owner
​
// Act
// Attempt to change owner from an unauthorized user
vm.prank(unauthorizedUser);
mysteryBox.changeOwner(newOwner);
​
// Assert
assertEq(mysteryBox.owner(), newOwner, "Owner should have been changed by unauthorized user");
}
​
function testChangeOwnerByOwner() public {
// Arrange
address newOwner = user1;
​
// Act
vm.prank(owner);
mysteryBox.changeOwner(newOwner);
​
// Assert
assertEq(mysteryBox.owner(), newOwner, "Owner should have been changed by the current owner");
}


Tools Used

Manual code review

Recommendations

Restrict the changeOwner function to be callable only by the current owner. Implement an access control mechanism to ensure that only authorized addresses can perform sensitive actions.

Example modification with access control:

function changeOwner(address _newOwner) public {
require(msg.sender == owner, "Only owner can change ownership");
require(_newOwner != address(0), "Invalid new 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!