PoolTogether V5: Part Deux - Jorgect's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngRelayAuction.sol#L131

Vulnerability details

Impact

An arbitrary user can set a wining number and set a _rewardRecipient to take the rewards

Proof of Concept

The contract RngRelayAuction is setting a rngAuctionRelayer in the constructor.

file: pt-v5-draw-auction/src/RngRelayAuction.sol
 /// @notice The relayer that RNG results must originate from.
    /// @dev Note that this may be a Remote Owner if relayed over an ERC-5164 bridge.
    address public immutable rngAuctionRelayer;

https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngRelayAuction.sol#L77C1-L79C46

file: pt-v5-draw-auction/src/RngRelayAuction.sol
constructor(
    PrizePool prizePool_,
    address _rngAuctionRelayer,
    uint64 auctionDurationSeconds_,
    uint64 auctionTargetTime_
  ) {
    ...
    rngAuctionRelayer = _rngAuctionRelayer;
    ...
  }

https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngRelayAuction.sol#L102C3-L121C4

However this relayer is never used, Now look the rngComplete:

file: pt-v5-draw-auction/src/RngRelayAuction.sol

  function rngComplete(
    uint256 _randomNumber,
    uint256 _rngCompletedAt,
    address _rewardRecipient,
    uint32 _sequenceId,
    AuctionResult calldata _rngAuctionResult
  ) external returns (bytes32) {
    if (_sequenceHasCompleted(_sequenceId)) revert SequenceAlreadyCompleted();
    uint64 _auctionElapsedSeconds = uint64(block.timestamp < _rngCompletedAt ? 0 : block.timestamp - _rngCompletedAt);
    if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired();
    ...
    uint32 drawId = prizePool.closeDraw(_randomNumber); //setting wining number
    ...
    for (uint8 i = 0; i < _rewards.length; i++) {
      uint104 _reward = uint104(_rewards[i]);
      if (_reward > 0) {
        prizePool.withdrawReserve(auctionResults[i].recipient, _reward); //sending the rewards
        emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward);
      }
    }


    return bytes32(uint(drawId));
  }

https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngRelayAuction.sol#L131C3-L177C1

We can see that this function is never checking for the caller allowing a user pass a ramdom number and pass a winners.

We can analize the calls that this function make to the prizePool contract:

• Closing the draw and setting a winningRandomNumber_ in the prizePool:

// this function is in the prizePool contract 
 function closeDraw(uint256 winningRandomNumber_) external onlyDrawManager returns (uint16) {

• Sending the rewards passing a winner in the prizePool

function withdrawReserve(address _to, uint104 _amount) external onlyDrawManager {

An user can pass those values for his convinience taking the reward for free.

Tools Used

manual

Recommended Mitigation Steps

The contract should check for the rngAuctionRelayer which is the only one that can call the rngComplete function

Assessed type

Access Control