Pot constructor does not dedupe i_players, so a duplicate address overwrites its own reward and inflates the closePot divisor
Summary
The Pot constructor builds the playersToRewards mapping by looping over i_players with no duplicate check. If the same address appears twice in players[], the second write overwrites the first (only the last reward survives for that address), while i_players.length still counts both entries. The inflated length is later used as the divisor in closePot, shrinking every claimant's share and stranding more residue.
Description
players[] and rewards[] are supplied at contest creation and trusted as-is. A duplicated player corrupts two independent things at once: the per-address reward (last-write-wins) and the player count used for distribution math.
Risk
A contest created with a duplicate player address mis-allocates rewards and worsens the residue-stranding already present in closePot. The duplicated player can only ever claim the surviving (last) reward, losing the earlier allocation, and the larger i_players.length further reduces the per-claimant share at close.
Vulnerability Details
src/Pot.sol constructor (L36-38):
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
If i_players = [A, A] with i_rewards = [100e18, 50e18], then playersToRewards[A] ends at 50e18 (the 100e18 is lost), but i_players.length == 2. In closePot, claimantCut = (remainingRewards - managerCut) / 2 even though there is effectively one unique player, compounding the divisor-mismatch stranding.
Impact
Reward loss for the duplicated address (100e18 -> 50e18 in the PoC).
Divisor inflation that strands additional residue at close.
Proof of Concept
test_HH4_duplicate_player_overwrites_and_inflates. Real forge output:
[PASS] test_HH4_duplicate_player_overwrites_and_inflates()
Assertions: playersToRewards[A] == 50e18 (the second, smaller value), not 100e18; i_players.length == 2 driving the inflated divisor.
Recommended Mitigation
Reject duplicate players at construction (or sum their rewards intentionally). A simple guard: track seen addresses and revert on a repeat, e.g. require players are unique before populating the mapping.
Note: this finding's precondition is owner-supplied input (players[] is set by the contest creator, who is trusted-but-bounded), so a judge may tier the likelihood lower.
Tools Used
Foundry (forge), manual review.
Lead Judging Commences
[H-03] [M1] `Pot::constructor` Overwrites Rewards for Duplicate Players, Leading to Incorrect Distribution
## Description The `for` loop inside the `Pot::constructor` override the `playersToRewards[i_players[i]]` with new reward `i_rewards[i]`.So if a player's address appears multiple times, the reward is overwritten rather than accumulated. This results in the player receiving only the reward from the last occurrence of their address in the array, ignoring prior rewards. ## Vulnerability Details **Proof of Concept:** 1. Suppose i_players contains \[0x123, 0x456, 0x123] and i_rewards contains \[100, 200, 300]. 2. The playersToRewards mapping will be updated as follows during construction: - For address 0x123 at index 0, reward is set to 300. - For address 0x456 at index 1, reward is set to 200. - For address 0x123 at index 2, reward is updated to 100. 3. As a result, the final reward for address 0x123 in playersToRewards will be 100, not 400 (300+100).This leads to incorrect and lower reward distributions. **Proof of Code (PoC):** place the following in the `TestMyCut.t.sol::TestMyCut` ```Solidity address player3 = makeAddr("player3"); address player4 = makeAddr("player4"); address player5 = makeAddr("player5"); address[] sixPlayersWithDuplicateOneAddress = [player1, player2, player3, player4, player1, player5]; uint256[] rewardForSixPlayers = [2, 3, 4, 5, 6, 7]; uint256 totalRewardForSixPlayers = 27; // 2+3+4+5+6+7 function test_ConstructorFailsInCorrectlyAssigningReward() public mintAndApproveTokens { for (uint256 i = 0; i < sixPlayersWithDuplicateOneAddress.length; i++) { console.log("Player: %s reward: %d", sixPlayersWithDuplicateOneAddress[i], rewardForSixPlayers[i]); } /** * player1 has two occurance in sixPlayersWithDuplicateOneAddress ( at index 0 and 4) * So it's expected reward should be 2+6 = 8 */ vm.startPrank(user); contest = ContestManager(conMan).createContest(sixPlayersWithDuplicateOneAddress, rewardForSixPlayers, IERC20(ERC20Mock(weth)), totalRewardForSixPlayers); ContestManager(conMan).fundContest(0); vm.stopPrank(); uint256 expectedRewardForPlayer1 = rewardForSixPlayers[0] + rewardForSixPlayers[4]; uint256 assignedRewardForPlaye1 = Pot(contest).checkCut(player1); console.log("Expected Reward For Player1: %d", expectedRewardForPlayer1); console.log("Assigned Reward For Player1: %d", assignedRewardForPlaye1); assert(assignedRewardForPlaye1 < expectedRewardForPlayer1); } ``` ## Impact The overall integrity of the reward distribution process is compromised. Players with multiple entries in the i_players\[] array will only receive the reward from their last occurrence in the array, leading to incorrect and lower reward distributions. ## Recommendations **Recommended Mitigation:** Aggregate the rewards for each player inside the constructor to ensure duplicate addresses accumulate rewards instead of overwriting them.This can be achieved by using the += operator in the loop that assigns rewards to players. ```diff for (uint256 i = 0; i < i_players.length; i++) { - playersToRewards[i_players[i]] = i_rewards[i]; + playersToRewards[i_players[i]] += i_rewards[i]; } ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.