code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Unfair Winner Selection in Prize Distribution Compromises Fairness in `PoolTogether` Protocol #368

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L104-L111 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L73-L96

Vulnerability details

Impact

The unique user hash is converted to a uint256 before being used in the prize distribution mechanism. Winners who deserve to win based on their unique user characteristics, tier and winning random number may not be accurately determined. The uint(hash) values may not be unique to each user potentially leading to the selection of incorrect winners

Proof of Concept

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L104-L111

The vulnerability arises from the conversion of a unique user hash to a uint256 which leads to non-uniqueness. The unique user hash is generated using the Keccak256 hash function, which is a cryptographically secure hash function. The hash function takes any input and produces a unique hash value. In the PoolTogether protocol, the unique user hash is used to identify each participant in the prize draw. Converting the unique user-specific hash into a uint256 implies that the uniqueness of the pseudorandom number is lost. This has a direct impact on the fairness and integrity of the winner selection process.

  /// @notice Calculates a pseudo-random number that is unique to the user, tier and winning random number.
  function calculatePseudoRandomNumber(....)

   return `uint256`(keccak256(abi.encode(_user, _tier, _prizeIndex, _winningRandomNumber)));

Tools Used

Manual analysis

Recommended Mitigation Steps

Explore alternative methods that preserve the uniqueness of the hash while maintaining randomness and security.

Conclusion:

Assessed type

en/de-code

code423n4 commented 1 year ago

Withdrawn by ro1sharkm