Constructor overwrites rewards for duplicate players — uses = not +=
Title: Constructor overwrites rewards for duplicate players — uses = not +=
Impact: High. Duplicate player entries lose all but their last reward — earlier rewards unclaimable.
Likelihood: Medium. Requires owner to pass duplicate addresses in the players array.
Reference Files: repos/src/Pot.sol:22-35
Description
The Pot constructor iterates over the i_players array and assigns rewards to the playersToRewards mapping using the = operator. If the same address appears multiple times in the i_players array, each subsequent assignment overwrites the previous one — the player receives only the reward from their last occurrence. The earlier reward values are permanently lost because no other address is mapped to claim them, yet remainingRewards accounts for the full sum.
The lost reward is accounted for in remainingRewards but no one can claim it — the tokens are distributed to other claimants during closePot, creating an unfair allocation where the duplicate player's intended total is silently reduced.
Risk
Impact: High. A player appearing twice in the array with rewards [100, 300] receives only 300 — losing 100 that becomes part of the unclaimed pool. If the owner intended to give them 400, they permanently receive 25% less than expected with no warning or error.
Likelihood: Medium. Requires the owner to accidentally include a duplicate address in the players array. While admin error, the silent overwrite gives no indication of the mistake — the transaction succeeds and the contract deploys normally with corrupted reward data.
With a 10-player contest where one player appears 3 times with rewards [50, 30, 20], they receive only 20 — losing 80 tokens to the unclaimed pool with no recovery possible.
Proof of Concept
The PoC deploys a Pot with a duplicate player and proves the reward mapping silently drops the earlier 100-token reward.
Recommended Mitigation
Using += accumulates rewards for duplicate entries instead of silently overwriting them, ensuring players receive the sum of all rewards assigned to their address.
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.