Title: Duplicate player addresses overwrite earlier rewards and cause reward loss
Severity: High
Scope Affected:
src/ContestManager.sol
ContestManager.createContest()
src/Pot.sol
Pot.constructor()
Pot.claimCut()
The normal behavior is that each reward entry assigned during contest creation should be claimable by the intended player.
The issue is that Pot stores rewards in a mapping from player address to reward amount. When the players array contains the same address more than once, the later reward overwrites the earlier reward. The duplicated player can claim only the last mapped reward, while the overwritten reward remains counted in remainingRewards and can become stuck or incorrectly redistributed.
Likelihood:
This occurs whenever contest creation includes the same player address more than once.
This occurs silently because neither ContestManager.createContest() nor Pot.constructor() validates uniqueness.
Impact:
A duplicated player loses any earlier reward entries overwritten by later entries.
The overwritten reward remains in pot accounting and can become stuck or distort close-time distribution.
The first 300 ether reward is overwritten by the second 700 ether reward. The player cannot claim the full 1_000 ether allocated to their duplicated entries.
Reject duplicate player addresses during contest creation or pot construction:
For larger arrays, use an enumerable set or off-chain validation with a Merkle tree design.
## 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]; } ```
The contest is live. Earn rewards by submitting a finding.
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsThe contest is complete and the rewards are being distributed.