Lines of code

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&gt;

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&gt;

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&gt;

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

---

The text was updated successfully, but these errors were encountered:

All reactions