The canStartDraw function may prevent auction draws from being started
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, draw auctions may be prevented from being started.
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. Thus, if canStartDraw returns false, they would not proceed to call startDraw.
The issue is that canStartDraw function may incorrectly return that a draw cannot be started, even when the startDraw function will not revert. Here is the inaccuracy:
The following check is made in canStartDraw:
_computeElapsedTime(drawClosesAt, block.timestamp) <= auctionDuration // the draw hasn't expired
This validation is inaccurate as if the previous request for randomness has failed, the next draw auction starts when the failed draw auction closed, not when the draw to be awarded closed. This makes the following scenario possible:
1/ First start draw auction completion attempt is made (through the startDraw function), with a duration of 4 hours
2/ The randomness request fails and a third party intends to make a second attempt, 3 hours after the first auction closed
3/ The third-party calls canStartDraw to make sure that a draw can be started and they will not lose funds by paying for the Witnet randomness fee
4/ canStartDraw returns false as _computeElapsedTime(drawClosesAt, block.timestamp) returns the difference between the current timestamp and the beginning of the first auction, which would be 7 hours (> 6 hours)
5/ No third party calls the startDraw function, due to the risk of losing funds when paying for the randomness request fee
Impact
Awarding certain draws will be skipped, due to the inaccuracy in canStartDraw.
If there has been a previous auction that did not complete due to a failed request for randomness, the validation above should compute the elapsed time between the end of the last auction and the current timestamp.
trachev
medium
The
canStartDrawfunction may prevent auction draws from being startedSummary
The
canStartDrawfunction inDrawManager.solis used to check if a start draw auction can be completed. If the function were to return erroneous data, draw auctions may be prevented from being started.Vulnerability Detail
In order for the
startDrawfunction 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
startDrawis intended to be called by third-party bots and users, looking to gain the reward of completing a draw auction, thecanStartDrawwill 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 tostartDrawfailed, they would have lost all of the funds used to pay for the Witnet fee. Thus, ifcanStartDrawreturns false, they would not proceed to callstartDraw.The issue is that
canStartDrawfunction may incorrectly return that a draw cannot be started, even when thestartDrawfunction will not revert. Here is the inaccuracy: The following check is made incanStartDraw:This validation is inaccurate as if the previous request for randomness has failed, the next draw auction starts when the failed draw auction closed, not when the draw to be awarded closed. This makes the following scenario possible:
1/ First start draw auction completion attempt is made (through the
startDrawfunction), with a duration of 4 hours 2/ The randomness request fails and a third party intends to make a second attempt, 3 hours after the first auction closed 3/ The third-party callscanStartDrawto make sure that a draw can be started and they will not lose funds by paying for the Witnet randomness fee 4/canStartDrawreturns false as_computeElapsedTime(drawClosesAt, block.timestamp)returns the difference between the current timestamp and the beginning of the first auction, which would be 7 hours (> 6 hours) 5/ No third party calls thestartDrawfunction, due to the risk of losing funds when paying for the randomness request feeImpact
Awarding certain draws will be skipped, due to the inaccuracy in
canStartDraw.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
If there has been a previous auction that did not complete due to a failed request for randomness, the validation above should compute the elapsed time between the end of the last auction and the current timestamp.
Duplicate of #129