code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Explanation

For the DrawAuction to be successfully completed, a specific sequence of actions must be followed, and each step holds significance. This demands a carefully designed execution flow that considers both the procedure's requirements and the authorized entities who can invoke these actions.

One pivotal function within this process is rngComplete(), which serves the purpose of finalizing the draw auction on the toChain and incorporating the generated random number into the prizePool. The code comments specify that only a relayer should have the privilege to call this function. Unfortunately, this crucial protocol was not adhered to, leading to an unintended vulnerability that allows anyone to execute the function as long as the sequence remains open.

Impact

Given the ability for the caller to input their own random number instead of waiting for the one generated by the RNG, several advantages are available to the msg.sender who exploits this vulnerability. Among these advantages, the most significant is the potential for financial gain. By designating their address as the rewardRecipient, the individual can direct the rewards to themselves.

    for (uint8 i = 0; i < _rewards.length; i++) {
      uint104 _reward = uint104(_rewards[i]);
      if (_reward > 0) {
        prizePool.withdrawReserve(auctionResults[i].recipient, _reward);
        emit AuctionRewardDistributed(_sequenceId, auctionResults[i].recipient, i, _reward);
      }
    }

Recommendation

Add a require state that ensures that RngRelayAuction::rngComplete() is only callable by the auction relayer

Assessed type

Access Control

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #82

c4-judge commented 1 year ago

HickupHH3 marked the issue as satisfactory

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