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

`Pot` constructor does not check that `rewards` and `players` have the same length, so extra rewards are silently ignored

theo_mc

Root + Impact

The constructor loops over players without checking that both arrays have the same length. If rewards is longer, its extra entries are ignored. Meanwhile, remainingRewards is set to the separately supplied totalRewards, which can exceed the rewards assigned to players.

Description

The constructor assigns rewards using this loop:

for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}

There is no check that players.length == rewards.length.

For example, the owner supplies two players, rewards [1, 2, 3], and totalRewards = 6. Only rewards 1 and 2 are assigned. The third reward is ignored, while remainingRewards is set to 6.

Checking that sum(rewards) == totalRewards would not catch this example: both values are 6.

If rewards is shorter than players, deployment reverts with an array out-of-bounds error. The silent omission occurs when rewards is longer.

Risk

Likelihood: Low

The trusted owner must accidentally supply arrays of different lengths. An ordinary player cannot create this configuration.

Impact: Low

Some of the funded tokens are not assigned to any player. The constructor does not report this, so the owner does not notice the mistake.
At closing, part of these tokens may be paid to the players who claimed, but the player the owner forgot to add still gets nothing.
The closing payments are also affected by the separate division issue (reported separately).

Proof of Concept

Add the following inside the existing TestMyCut contract in test/TestMyCut.t.sol. It uses the existing setup, imports and the mintAndApproveTokens modifier.

function test_PoC_claimableSumLessThanRemaining() public mintAndApproveTokens {
address[] memory p = new address[](2);
p[0] = player1;
p[1] = player2;
​
uint256[] memory r = new uint256[](3); // one reward more than players
r[0] = 1;
r[1] = 2;
r[2] = 3;
​
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
p,
r,
IERC20(ERC20Mock(weth)),
6
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
Pot pot = Pot(contest);
uint256 claimable = pot.checkCut(player1) + pot.checkCut(player2);
​
console.log("players can claim in total:", claimable);
console.log("remainingRewards:", pot.getRemainingRewards());
​
assertEq(claimable, 3);
assertEq(pot.getRemainingRewards(), 6);
assertLt(claimable, pot.getRemainingRewards());
}

Run from the repository root:

forge test --mt test_PoC_claimableSumLessThanRemaining -vv

Output:

[PASS] test_PoC_claimableSumLessThanRemaining() (gas: 1057764)
...
players can claim in total: 3
remainingRewards: 6

Recommended Mitigation

Check the lengths when the Pot is created, so a wrong input fails immediately:

+ error Pot__LengthMismatch();
​
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ if (players.length != rewards.length) revert Pot__LengthMismatch();
...
}
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!