Duplicate player addresses overwrite earlier rewards, locking tokens in the Pot
Description
The Pot constructor uses direct = assignment when mapping player addresses to rewards. If the same address appears more than once in the players array, the later entry overwrites the earlier one. The overwritten reward amount can never be claimed by anyone, but it was included in totalRewards and funded into the Pot.
Vulnerability Details
The constructor loop assigns rewards using =, not +=:
If players = [Alice, Bob, Alice] with rewards = [500, 300, 200], the mapping state after the loop is:
| Iteration | playersToRewards[Alice] |
playersToRewards[Bob] |
|---|---|---|
| i=0 | 500 | 0 |
| i=1 | 500 | 300 |
| i=2 | 200 (overwrites 500) | 300 |
Alice can only claim 200 tokens. The original 500 allocation is erased from the mapping. The totalRewards was set to 1000 (500 + 300 + 200), and fundContest() transfers 1000 tokens into the Pot. But only 500 tokens (200 + 300) are claimable. The remaining 500 are permanently locked — the Pot has no withdrawal or rescue function.
The claimants array also grows incorrectly. Alice appears twice in i_players, so when she calls claimCut(), she is pushed to claimants once. But the i_players.length used in closePot() for the surplus denominator counts her twice, further compounding the wrong-denominator bug (H-01).
Risk
Likelihood:
This requires the trusted admin to accidentally include a duplicate address in the players array when calling
createContest(). While not an attacker-exploitable vector, the lack of any deduplication check means a simple copy-paste error during contest setup silently creates a broken reward structure.
Impact:
The overwritten player receives less than their intended reward with no recourse. The difference between the original and overwritten amount is permanently locked in the Pot. For large contests with many players, a single duplicate could lock thousands of tokens.
PoC
The test creates a contest where Alice appears twice in the players array — once with a 1000-token reward and once with 200. The second entry overwrites the first. Alice claims 200, but the remaining 800 tokens that were funded based on the original totalRewards calculation are permanently stuck.
Recommendations
Check for duplicate addresses during construction to reject misconfigured inputs immediately. Since playersToRewards defaults to 0, any non-zero value means the address was already assigned a reward.
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.