Duplicate addresses in the players array silently overwrite earlier rewards in the constructor, permanently locking the overwritten amounts and inflating the close-time distribution divisor
Root + Impact
Description
-
Normally, each address in players maps to exactly one reward in playersToRewards, and each unique player counts once in the close-time divisor.
-
With duplicates the mapping assignment overwrites: only the last reward for that address is claimable, the earlier amounts are unclaimable by anyone (they stay inside remainingRewards but no address maps to them), and i_players.length counts the duplicates — so the divisor in closePot is inflated (compounding V2), and the residual is permanently stranded.
Risk
Likelihood:
Any duplicated winner row in the configuration array (copy-paste of a payouts sheet, multi-round winners listed twice) is accepted silently — creation, funding and even the duplicate's own claim all succeed, so the error is invisible until close time.
Impact:
-
Verified: players [p1, p1, p2] with rewards [5, 4, 1] (total 10) — p1's 5 is silently lost (only 4 claimable), and at close the remainder divides by 3 instead of 2, so p1 receives 6 in total while 4 stays permanently locked.
-
The overwritten amounts belong to the pot's budget but map to no claimant — combined with V2's wrong divisor and V10's dust, the stranded share is unrecoverable.
Proof of Concept
PoC explanation: The test quantifies both halves of the duplicate-entry problem. Setup: the admin creates a pot with players [p1, p1, p2], rewards [5, 4, 1], totalRewards = 10 — accepted silently. First observation: checkCut(p1) returns 4, proving the constructor's last-write-wins mapping silently DISCARDED p1's first reward of 5 (no address maps to it ever again). Step: p1 claims his 4; at close, remainder = 6 is divided by the DUPLICATE-INFLATED divisor i_players.length = 3 (instead of 2 unique players), yielding claimantCut = 2. Final assertions: p1's total is 6 (4 claimed + 2 close-share — he lost 4 of what his 5-entry should entitle), and 4 WETH are permanently locked in the pot. The same entry both destroys the overwritten reward and dilutes the close-time distribution (compounding V2's wrong divisor). Run with: forge test --match-test test_POC8_DuplicatePlayer_OverwriteAndDilution -vv (PASS; asserts checkCut = 4, p1 total 6, residual 4).
Recommended Mitigation
⚠ Combination note: rejecting rewards[i] == 0 explicitly is required for the != 0 duplicate check to be sound (a legitimate 0-reward entry would otherwise false-positive as a duplicate); fixing uniqueness here does NOT fix the wrong-divisor bug (V2) — i_players.length becomes unique-count but still not claimants.length.
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.