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

12 stars 7 forks source link

Unsecure and predictable random number generation in closeDraw.winningRandomNumber_() #463

Closed code423n4 closed 12 months ago

code423n4 commented 12 months ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L348-#L387

Vulnerability details

Impact

Unsecure and predictable random number generation in closeDraw.winningRandomNumber_() can lead to external influence by malicious attackers. Leading to undermining of the fairness and security and unpredictability of the draw function. Both the timestamp and the block hash can be influenced by miners to some degree.

Proof of Concept

File: PrizePool.sol Code Link: https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L348C3-L387C1 Code: function closeDraw(uint256 winningRandomNumber) external onlyDrawManager returns (uint16) { // check winning random number if (winningRandomNumber == 0) { revert RandomNumberIsZero(); } if (block.timestamp < _openDrawEndsAt()) { revert DrawNotFinished(_openDrawEndsAt(), uint64(block.timestamp)); }

uint8 _numTiers = numberOfTiers;
uint8 _nextNumberOfTiers = _numTiers;

if (lastClosedDrawId != 0) {
  _nextNumberOfTiers = _computeNextNumberOfTiers(_numTiers);
}

uint64 openDrawStartedAt_ = _openDrawStartedAt();

_nextDraw(_nextNumberOfTiers, uint96(_contributionsForDraw(lastClosedDrawId + 1)));

_winningRandomNumber = winningRandomNumber_;
claimCount = 0;
canaryClaimCount = 0;
largestTierClaimed = 0;
_lastClosedDrawStartedAt = openDrawStartedAt_;
_lastClosedDrawAwardedAt = uint64(block.timestamp);

emit DrawClosed(
  lastClosedDrawId,
  winningRandomNumber_,
  _numTiers,
  _nextNumberOfTiers,
  _reserve,
  prizeTokenPerShare,
  _lastClosedDrawStartedAt
);

return lastClosedDrawId;

}

Tools Used

Manual Review

Recommended Mitigation Steps

Do not use block.timestamp as a source of randomness, instead use various multiple time sources and external consensus mechanisms. The current block timestamp must be strictly larger than the timestamp of the last block, but the only guarantee is that it will be somewhere between the timestamps of two consecutive blocks in the canonical chain.

Assessed type

Timing

c4-judge commented 12 months ago

Picodes marked the issue as unsatisfactory: Insufficient quality