The changeOwner()
function in the MysteryBox contract lacks proper access control, allowing any address to become the owner of the contract. This critical vulnerability can lead to a complete takeover of the contract and loss of funds.
The changeOwner()
function is implemented as follows:
This function is public
, meaning it can be called by any external address. There are no checks to ensure that only the current owner can call this function. As a result, any address can call this function and set themselves (or any other address) as the new owner of the contract.
The impact of this vulnerability is severe:
Unauthorized Contract Takeover: Any malicious actor can become the owner of the contract at will.
Fund Theft: The new owner can call withdrawFunds()
to steal all the ETH stored in the contract.
Price Manipulation: The new owner can manipulate the box price using setBoxPrice()
, potentially draining users' funds.
Reward Manipulation: The new owner can add or modify rewards, potentially creating worthless rewards or removing valuable ones.
Given that the owner has complete control over the contract's crucial functions, this vulnerability effectively allows for a complete compromise of the contract's integrity and user funds.
This vulnerability was discovered through manual code review. No specific tools were required to identify this issue.
To fix this vulnerability, implement proper access control for the changeOwner()
function. Here's a recommended implementation:
This implementation:
Ensures only the current owner can change ownership.
Prevents setting the owner to the zero address.
Emits an event for transparency (you'll need to define this event).
Additionally, consider implementing a two-step ownership transfer process for added security, where the new owner must accept ownership in a separate transaction.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.