Summary

A security review of the Pot contract revealed a significant vulnerability in handling duplicate player addresses during contract initialization. This issue could lead to incorrect reward distribution and potential loss of funds.

Vulnerability Details

The constructor of the Pot contract doesn't check for or properly handle duplicate addresses in the players array, leading to potential overwriting of reward values:

If the same address appears multiple times in the players array, each subsequent occurrence will overwrite the previous reward value in the playersToRewards mapping.

Impact

The presence of duplicate player addresses can have several serious consequences:

1. Incorrect Reward Distribution: Players with duplicate addresses will only receive the last reward amount assigned to them, potentially losing out on earlier, possibly larger rewards.

2. Fund Loss: Some rewards might become inaccessible if earlier entries for a duplicate address are overwritten, effectively locking those funds in the contract.

3. Inconsistent State: The sum of rewards in playersToRewards might not match the i_totalRewards, leading to accounting discrepancies.

4. Unfair Advantage: If intentional, this could be exploited to give certain players multiple entries, increasing their chances of receiving rewards unfairly.

Example scenario:

In this scenario, 100 tokens are effectively lost and Alice's initial reward is overwritten.

Tools Used

Manual code review

AI for report.

Recommendations

1. Implement Duplicate Address Check:
   Use a data structure like OpenZeppelin's EnumerableSet to efficiently track and handle unique addresses:

   import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

   ​

   contract Pot is Ownable(msg.sender) {

   using EnumerableSet for EnumerableSet.AddressSet;

   ​

   EnumerableSet.AddressSet private playerSet;

   ​

   constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {

   require(players.length == rewards.length, "Players and rewards must have the same length");

   uint256 totalAssignedRewards = 0;

   for (uint256 i = 0; i < players.length; i++) {

   if (playerSet.add(players[i])) {

   // New unique player

   playersToRewards[players[i]] = rewards[i];

   } else {

   // Duplicate player - summing rewards

   playersToRewards[players[i]] += rewards[i];

   }

   totalAssignedRewards += rewards[i];

   }

   require(totalAssignedRewards == totalRewards, "Total assigned rewards must match total rewards");

   ​

   // ... rest of the constructor ...

   }

   ​

   // ... rest of the contract ...

   }

2. Validate Total Rewards: Ensure that the sum of all rewards matches the expected total rewards to prevent any discrepancies.

3. Event Emission: Consider emitting an event when a duplicate address is detected, allowing for off-chain monitoring and auditing.

4. Clear Documentation: Clearly document the behavior for duplicate addresses in the contract's comments and external documentation.

5. Unit Testing: Implement comprehensive unit tests that cover scenarios with duplicate addresses to ensure the contract behaves as expected.

By implementing these recommendations, the contract will handle duplicate player addresses safely and transparently, preventing potential fund loss and ensuring fair reward distribution.