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

Possible inconsistency in `players` and `rewards` arrays length prevents contest creation

valya

Impact: Medium
Likelihood: Medium

Root + Impact

Description

  • It is possible to pass arrays address[] memory players and uint256[] memory rewards of different length as params to ContestManager::createContest(), where the lengths are never validated.

  • Later these arrays are passed to a Pot.sol constructor, where for each player from address[] memory players array a corresponding reward is assigned from a uint256[] memory rewards array and stored in a mapping mapping(address => uint256) private playersToRewards.

Here is ContestManager::createContest():

@> function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}

Here is Pot.sol constructor:

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
​
// i_token.transfer(address(this), i_totalRewards);
​
for (uint256 i = 0; i < i_players.length; i++) {
@> playersToRewards[i_players[i]] = i_rewards[i];
}
}

Risk

Likelihood:

  • The issue will occur when amount of members in players array is more than an amount of members in rewards array in params of ContestManager::createContest

Impact:

  • It will not be possible to create a Pot, the transaction will revert but gas for the previous actions in createContest() function and Pot.sol constructor will be vasted.

Proof of Concept

Please, add the following test test_revertsWhen_playersAndRewardsArraysLengthIsDifferent to TestMyCut.t.sol.
The amount of players is less than amount of rewards in arrays - players.length > rewards.length.

address player3 = makeAddr("player3");
address player4 = makeAddr("player4");
address[] playersLocal = [player1, player2, player3, player4];
​
function test_revertsWhen_playersAndRewardsArraysLengthIsDifferent() public mintAndApproveTokens {
uint256 playersLocalLength = playersLocal.length;
rewards = [500, 500, 500];
uint256 rewardsLength = rewards.length;
console.log("playersLocalLength ", playersLocalLength.toString()); // 4
console.log("rewardsLength ", rewardsLength.toString()); // 3
assertLt(rewardsLength, playersLocalLength);
totalRewards = 2500;
​
vm.prank(user);
vm.expectRevert(stdError.indexOOBError);// fail with 'panic: array out-of-bounds access'
contest = ContestManager(conMan).createContest(playersLocal, rewards, IERC20(ERC20Mock(weth)), totalRewards);
}

Recommended Mitigation

Check length of both arrays and revert if the lengths are not equal:

+error ContestManager__PlayersAmountIsNotEqualRewardsAmount();
​
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
+ require(players.length == rewards.length, ContestManager__PlayersAmountIsNotEqualRewardsAmount());
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours 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!