MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

[M-3] `ContestManager::closeContest` does not check that param `address contract` is an approved contest, and that address is used to call `ContestManager::_closeContest`, opening potential to calls to unapproved contracts

datadata

[M-3] ContestManager::closeContest does not check that param address contract is an approved contest, and that address is used to call ContestManager::_closeContest, opening potential to calls to unapproved contracts

Description

ContestManager::closeContest does not check that param address contract is an approved contest, and that address is used to call ContestManager::_closeContest, opening potential to calls to unapproved contracts. The call can happen so long as the unapproved contract has a function with the same function signature as Pot::closePot.

@> function closeContest(address contest) public onlyOwner {
@> _closeContest(contest);
}
​
@> function _closeContest(address contest) internal {
@> Pot pot = Pot(contest);
@> pot.closePot();
}

Risk

Likelihood:

This will happen any time the address of a contract is passed to ContestManager::closeContest that has a function that matches the function signature of Pot::closePot.

Impact:

The impact is that a given address is not checked as a valid contest, which means that a contract using the same function signature as Pot::closePot can be called, opening the door to malicious activity.

Proof of Concept

We define a malicious contract with the same function signature as Pot::closePot, and which targets ContestManager.

contract MaliciousContract {
ContestManager public target;
​
constructor(address _target) {
target = ContestManager(_target);
}
​
// mimics Pot::closePot
function closePot() external {
console.log("fake closePot called");
}
}

Our test function deploys the malicious contract fakePot, and the user calls ContestManager::closeContest with the param being the address of fakePot. This calls ContestManager::_closeContest, which calls fakePot's function MaliciousContract::closePot, which logs "fake closePot called", showing that the function runs. The test function passes.

function testCloseContestDoesNotCheckAddress() public mintAndApproveTokens {
// Arrange
// Deploy non-contest contract with conMan passed as address of target
MaliciousContract fakePot = new MaliciousContract(conMan);
​
// user deploys new contest
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
players,
rewards,
IERC20(ERC20Mock(weth)),
totalRewards
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
// Act / Assert
// user, who is the owner of contest, calls closeContest with address of maliciousPot
vm.startPrank(user);
ContestManager(conMan).closeContest(address(fakePot));
vm.stopPrank();
}

Recommended Mitigation

We recommend that a mapping of addresses to booleans be created to define if an address is a contest in ContestManager.

+ mapping (address => bool) public addressToContestBool;

The mapping will allow for constant-time look-up to verify that a given address is a contest. This mapping can be updated in ContestManager::createContest:

+ addressToContestBool[(address(pot))] = true;

We recommend that a custom error be added to ContestManager:

+ error ContestManager__addressIsNotAContest();
function closeContest(address contest) public onlyOwner {
+ if (!addresstoContestBool[contest]) {
+ revert ContestManager__addressIsNotAContest();
+ }
_closeContest(contest);
}
​
function _closeContest(address contest) internal {
+ if (!addresstoContestBool[contest]) {
+ revert ContestManager__addressIsNotAContest();
+ }
Pot pot = Pot(contest);
pot.closePot();
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 44 minutes ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!