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

[M-2] `ContestManager::getContestRemainingRewards` does not check that param `address contract` is an approved contest, allowing function calls on unapproved contracts

datadata

[M-2] ContestManager::getContestRemainingRewards does not check that param address contract is an approved contest, allowing function calls on unapproved contracts

Description

ContestManager::getContestRemainingRewards does not check that param address contract is an approved contest, allowing function calls on unapproved contracts. The logic currently puts contests in array ContestManager::contests, which could be checked, although we recommend the use of a mapping to do this check. See mitigation section.

function getContestRemainingRewards(
@> address contest
) public view returns (uint256) {
@> Pot pot = Pot(contest);
@> return pot.getRemainingRewards();
}

Risk

Likelihood:

This will happen any time an address is is passed to ContestManager::getContestRemainingRewards that has a function that matches the function signature of Pot::getRemainingRewards.

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::getRemainingRewards can be called, returning an incorrect value to ContestManager::getContestRemainingRewards, instead of this function reverting.

Proof of Concept

We define a fake contract DummyContract in the test file test/TestMyCut.t.sol, which has a function with an identical function signature to Pot::getRemainingRewards.

contract DummyContract {
function getRemainingRewards() external pure returns (uint256) {
uint256 fakeNumber = 999999;
return fakeNumber;
}
}

In our test function, we deploy a fake contest contract fakePot, and its address is passed to ContestManager::getContestRemainingRewards, which calls fakePot::getRemainingRewards, which returns fakeNumber, thus inserting incorrect information into the protocol.

function testContestManagerFunctionDoesNotCheckAddress()
public
mintAndApproveTokens
{
// Arrange
// Deploy non-contest contract
DummyContract fakePot = new DummyContract();
​
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
players,
rewards,
IERC20(ERC20Mock(weth)),
totalRewards
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
// Act
// pass address of fakePot to ContestManager::getContestTotalRewards.
// fakePot has a function with same signature as Pot::getRemainingRewards
uint256 remainingRewards = ContestManager(conMan)
.getContestRemainingRewards(address(fakePot));
​
console.log("remainingRewards: ", remainingRewards);
​
// Assert
uint256 expectedFakeRemainingRewards = 999999;
assertEq(remainingRewards, expectedFakeRemainingRewards);
}

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();

We recommend that the mapping be checked, and if the check does not return true, the custom error be thrown.

function getContestRemainingRewards(
address contest
) public view returns (uint256) {
+ if (!addresstoContestBool[contest]) {
+ revert ContestManager__addressIsNotAContest();
+ }
Pot pot = Pot(contest);
return pot.getRemainingRewards();
}
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!