MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Incorrect initialization of the owner in the Pot contract.

elprofesor

Summary

Essentially, the user calls the constructor from the Pot contract via the ContestManager contract and the createContest function to create a contest. The problem is that in the Pot contract, the owner is initialized as msg.sender. Since the constructor from the Pot contract is called this way, msg.sender is not the user creating the contest, but rather the ContestManager contract.

Vulnerability Details

ContestManager::createContest

Pot::constructor

Place the following test in the test/TestMyCut.s.sol file:

function testPotOwner() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
address potOwner = Pot(contest).owner();
​
assert(potOwner!=user);
assertEq(potOwner,address(conMan));
​
console.log("-----------------------------------------------------------------");
console.log("Real owner of pot:",potOwner);
console.log("User who start pot:",user);
console.log("Contest manager:",address(conMan));
}

The test does the following:

  1. The user starts a prank session to simulate a user's actions.

  2. The user calls the createContest function on the ContestManager contract, which in turn calls the Pot contract's constructor.

  3. The test then checks the owner of the Pot contract and asserts that it is not the user, but rather the ContestManager contract.

The issue lies in the Pot contract's constructor, where the owner is initialized as msg.sender. Since the Pot contract's constructor is called from the ContestManager contract, msg.sender is the ContestManager contract, not the user who is creating the contest.

Impact

The owner of the Pot contract is not the user who created the contest, but the ContestManager contract. This could lead to the not intended behavior.

Tools Used

Manual Code Review, Foundry Test

Recommendations

The recommended mitigation is to pass the user's address as a parameter to the Pot contract's constructor, instead of using msg.sender to initialize the owner. This way, the user who creates the contest will be the owner of the Pot contract, as expected.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.