Mystery Box

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

Critical Access Control Vulnerability in changeOwner Function

Summary

The MysteryBox contract contains a critical vulnerability in the changeOwner function that allows any address to take control of the contract by changing its owner without authorization.

Vulnerability Details

The changeOwner function lacks proper access control checks. Any external address can call this function and change the contract owner, completely undermining the contract's governance model.

Vulnerable Code:

(MysteryBox.sol#111-113)

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

This function allows any address to change the contract owner without any restrictions.

Impact

This vulnerability has a critical impact on the security and integrity of the entire contract. An attacker could take full control of the contract.

Tools Used

slither .

INFO:Detectors:
Function MysteryBox.changeOwner(address) (MysteryBox.sol#111-113) is a non-protected setter owner is written
Reference: https://github.com/pessimistic-io/slitherin/blob/master/docs/unprotected_setter.md

forge foundry

forge test --match-test testChangeOwnerWeakAccessControl -vvvv

function testChangeOwnerWeakAccessControl() public {
address initialOwner = mysteryBox.owner();
address nonOwner = address(0x1234);
address newOwner = address(0x5678);
// Non-owner attempts to change the owner
vm.prank(nonOwner);
mysteryBox.changeOwner(newOwner);
// Check that the owner has indeed changed
assertEq(mysteryBox.owner(), newOwner, "Non-owner was able to change the contract owner");
assertNotEq(mysteryBox.owner(), initialOwner, "Owner should have changed");
}

Recommendations

Implement proper access control on the changeOwner function, restricting it to the current owner only.

Consider using OpenZeppelin's Ownable contract for standardized ownership management.

Add events to log ownership changes for transparency.

Implement a two-step ownership transfer process for additional security.

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