trachev - The `canStartDraw` function may return wrong data, causing loss of funds

[sherlock-admin4]

trachev

medium

The canStartDraw function may return wrong data, causing loss of funds

Summary

The canStartDraw function in DrawManager.sol is used to check if a start draw auction can be completed. If the function were to return erroneous data, third-party bots and users may be under risk of a loss of funds.

Vulnerability Detail

In order for the startDraw function to complete successfully, the caller is required to have made a call to Witnet in order to make a request for randomness. This request is not free as it requires a fee to be paid.

Furthermore, as startDraw is intended to be called by third-party bots and users, looking to gain the reward of completing a draw auction, the canStartDraw will be used for them to make sure that making a request to Witnet and paying the necessary fee would be worthwhile. In other words, if they were to complete the request for randomness and after that their call to startDraw failed, they would have lost all of the funds used to pay for the Witnet fee.

This can occur when canStartDraw returns true instead of false, due to a mistake. Currently, there is an inaccuracy in the canStartDraw function, that may cause the function to return wrong data.

When drawId == lastStartDrawAuction.drawId it is made sure that:

(rng.isRequestFailed(lastStartDrawAuction.rngRequestId) && _startDrawAuctions.length <= maxRetries)

The issue is that it also needs to be checked whether block.number != rng.requestedAtBlock(lastRequest.rngRequestId), as if the two values are equal, canStartDraw would return true, but startDraw would revert:

} else if (block.number == rng.requestedAtBlock(lastRequest.rngRequestId)) { // requests cannot be reused
  revert StaleRngRequest();
}

Impact

This issue would cause third parties to make pointless requests for randomness, even when the startDraw function cannot be called, leading to a loss of funds, due to the fees paid to Witnet.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-draw-manager/src/DrawManager.sol#L277-L291

Tool used

Manual Review

Recommendation

Include the block.number != rng.requestedAtBlock(lastRequest.rngRequestId) check in the canStartDraw function:

(
      // if we're on a new draw
      drawId != lastStartDrawAuction.drawId ||
      // OR we're on the same draw, but the request has failed and we haven't retried too many times
      (rng.isRequestFailed(lastStartDrawAuction.rngRequestId) && _startDrawAuctions.length <= maxRetries && block.number != 
      rng.requestedAtBlock(lastRequest.rngRequestId))
) 

Duplicate of #129