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

`Pot` constructor does not enforce `players.length == rewards.length`; extra `rewards` entries are silently ignored, masking misconfigured Pots

cdude

Root + Impact

Description

  • The Pot constructor zips players and rewards by iterating up to i_players.length. When rewards.length > players.length, the surplus entries are silently dropped with no event and no revert. When rewards.length < players.length, the loop reverts with a generic out-of-bounds read — but only at the moment of deployment, providing no clear failure signal.

  • The result is that a misconfigured input (off-by-one in a deployment script, an extra trailing entry, etc.) creates a Pot whose effective rewards differ from what the admin intended, with no on-chain trace of the discrepancy.

// src/Pot.sol :: constructor
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
// ...
@> for (uint256 i = 0; i < i_players.length; i++) { // bound is players.length only
@> playersToRewards[i_players[i]] = i_rewards[i]; // silently ignores rewards[players.length..]
}
}

Risk

Likelihood:

  • Requires an admin (Trusted) misconfiguration. Realistic scenarios: a CSV with an extra row, a script that builds two arrays from different sources, or copy-paste artifacts.

  • The misconfiguration is silent at deploy time — there is no event, no return value, no warning — so it propagates to production unless the admin manually inspects each playersToRewards mapping after deployment.
    Impact:

  • The effective rewards configuration differs from the intended one. With rewards = [1000, 1000, 9999, 9999] and players = [alice, bob], the 9999 entries are simply dropped.

  • Combined with finding #4 (no sum(rewards) <= totalRewards check), the silent discard also opens the door to a Pot that appears over-funded but is in fact under-allocated, or vice versa, with no on-chain way to tell.

Proof of Concept

function test_excessRewardsArrayElementsIgnored() public {
address[] memory players = new address[](2);
players[0] = alice; players[1] = bob;
uint256[] memory rewards = new uint256[](4); // 2 extras
rewards[0] = 1000e18; rewards[1] = 1000e18;
rewards[2] = 9999e18; rewards[3] = 9999e18;
vm.startPrank(owner);
address potAddr = manager.createContest(players, rewards, IERC20(address(token)), 2000e18);
manager.fundContest(0);
vm.stopPrank();
Pot pot = Pot(potAddr);
assertEq(pot.checkCut(alice), 1000e18);
assertEq(pot.checkCut(bob), 1000e18);
// The 9999 entries were silently dropped — no revert, no event.
}

Run with forge test --match-test test_excessRewardsArrayElementsIgnored -vvv. Test passes.

Recommended Mitigation

Require strict length equality in the constructor (and ideally also in ContestManager.createContest).

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ require(players.length == rewards.length, "Pot: players/rewards length mismatch");
i_players = players;
i_rewards = rewards;
// ...
}
Updates

Lead Judging Commences

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