Mystery Box

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

Unrestricted Access to changeOwner Function Allows Arbitrary Ownership Transfer

Summary

The changeOwner function in the MysteryBox contract lacks proper access control, enabling any user to transfer ownership of the contract to an arbitrary address. This vulnerability allows malicious actors to assume control over owner-restricted functionalities, posing a significant security risk.

Vulnerability Details

The changeOwner function in src/MysteryBox.sol does not enforce any restrictions on who can call it. As a result, any user can invoke this function to change the contract's owner to their own address or another address of their choosing. This flaw effectively bypasses the intended access controls, granting unauthorized users the ability to perform privileged actions within the contract.

PoC

The following test case demonstrates how an unauthorized user can exploit the changeOwner function to transfer ownership of the contract.

function testChangeOwner_Vulnerability() public {
address attacker = user1; // Assume user1 is not the owner
// Attacker attempts to change the owner to their own address
vm.prank(attacker);
mysteryBox.changeOwner(attacker);
// Verify that the owner has been changed to the attacker's address
assertEq(mysteryBox.owner(), attacker, "Ownership was not transferred as expected");
}

Impact

High Impact

An attacker can withdraw all funds from the contract by leveraging ownership privileges.

Users may lose confidence in the protocol's security, leading to reputational damage.

Malicious changes to ownership can disrupt the intended functionality and administration of the contract.

Tools Used

Manual Code Review

Remix IDE

Recommendations

Implement Access Control:

Restrict the changeOwner function to be callable only by the current owner. This can be achieved by adding a require statement to verify the caller's identity.

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

Implement Proper Access Control Modifiers

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!