finishDraw will fail in many occurences due to unpredictable calculations
Summary
The finishDraw function in DrawManager.sol is used to initiate the awarding of a draw and pay out rewards to the third parties that completed the start and finish draw auctions. The issue is that the process of rewarding the third parties will cause the function to revert in many common occurrences.
Vulnerability Detail
The rewards paid out to the third parties that completed the start and finish draw auctions are derived as fractions of the reserve of the Prize Pool. Essentially, the issue is that the fractions may become more than 100% of the reserve, causing the function to revert, and for the draw to not be able to be awarded. The issue becomes even more likely to occur and extremely difficult to avoid when a start draw auction's request for randomness fails, and another auction needs to be completed.
Here is a coded POC, using the RewardLib.fractionalReward function, that calculates what fraction of the reserve each reward will take up, the POC uses the initial values for lastStartDrawFraction and lastFinishDrawFraction, which are likely to increase in production:
function testFractionsInaccuracy() external {
//This are the default values that are intended to be used by the protocol
uint256 _auctionDuration = 6 hours;
uint256 _auctionTargetTime = 1 hours;
UD2x18 _targetTimeFraction = (
intoUD2x18(convert(uint256(_auctionTargetTime)).div(convert(_auctionDuration)))
);
UD2x18 lastStartDrawFraction = UD2x18.wrap(0.1e18);
UD2x18 lastFinishDrawFraction = UD2x18.wrap(0.2e18);
UD2x18 firstStartFraction = fractionalReward(5 hours, uint48(_auctionDuration), _targetTimeFraction, lastStartDrawFraction);
console.log(firstStartFraction.unwrap());
UD2x18 finishFraction = fractionalReward(4 hours, uint48(_auctionDuration), _targetTimeFraction, lastFinishDrawFraction);
console.log(finishFraction.unwrap());
//We see that the accumulated fractions are more than 100%, which will cause the total rewards to be more than the reserve
console.log(1e18 >= firstStartFraction.unwrap() + finishFraction.unwrap());
//Result:
//firstStartFraction: 675999999999999998 == 67%
//finishFraction: 487999999999999998 == 48%
//1e18 >= firstStartFraction.unwrap() + finishFraction.unwrap(): false, as 1e18 would be 100% of the reserve
}
function fractionalReward(
uint48 _elapsedTime,
uint48 _auctionDuration,
UD2x18 _targetTimeFraction,
UD2x18 _targetRewardFraction
) internal pure returns (UD2x18) {
UD60x18 x = convert(_elapsedTime).div(convert(_auctionDuration)); //time of auction / 6 hours
UD60x18 t = UD60x18.wrap(_targetTimeFraction.unwrap()); //1/6
UD60x18 r = UD60x18.wrap(_targetRewardFraction.unwrap()); //0.1e18
UD60x18 rewardFraction;
if (x.gt(t)) {
//if the elapsed time fraction is more than the target tume fraction, therefore elapsed time is less than target time
UD60x18 tDelta = x.sub(t); //t delta = elapsed fraction - target fraction
UD60x18 oneMinusT = convert(1).sub(t); //1e18 - target time fraction
rewardFraction = r.add( //_target r fraction + ((1e18 - r) * tdelta * tdelta / oneminust / oneminust)
convert(1).sub(r).mul(tDelta).mul(tDelta).div(oneMinusT).div(oneMinusT)
);
} else {
UD60x18 tDelta = t.sub(x);
rewardFraction = r.sub(r.mul(tDelta).mul(tDelta).div(t).div(t));
}
return rewardFraction.intoUD2x18();
}
This coded POC illustrates a real scenario that is likely-to-occur, calculating the fraction of the reserve that the start and finish auctions will need to use. As it can be seen, the fractions become more than 100% of the reserve, which will later cause a revert. The two functions can be pasted in any testing file, as long as the required imports are implemented. The fractionalReward function is just a copy of the RewardLib.fractionalReward function.
As a disclaimer, this is not the only scenario where the issue occurs, there are several other likely-to-occur combinations of lastStartDrawFraction, lastFinishDrawFraction, number of auctions and elapsed auction times, which will cause an underflow of the reserve. For instance, in the POC above if there are two start auctions both elapsing 5 hours, the fractions would have outgrown the reserve, even before adding the finish draw auction.
There are several ways to resolve the issue. One solution would be to add a maximum value of the fractions of the reserve that each auction may utilize for rewards. For example, if there are two start auctions and one finish auction, each auction reward is limitied to a third of the reserve.
trachev
high
finishDrawwill fail in many occurences due to unpredictable calculationsSummary
The
finishDrawfunction inDrawManager.solis used to initiate the awarding of a draw and pay out rewards to the third parties that completed the start and finish draw auctions. The issue is that the process of rewarding the third parties will cause the function to revert in many common occurrences.Vulnerability Detail
The rewards paid out to the third parties that completed the start and finish draw auctions are derived as fractions of the reserve of the Prize Pool. Essentially, the issue is that the fractions may become more than 100% of the reserve, causing the function to revert, and for the draw to not be able to be awarded. The issue becomes even more likely to occur and extremely difficult to avoid when a start draw auction's request for randomness fails, and another auction needs to be completed. Here is a coded POC, using the
RewardLib.fractionalRewardfunction, that calculates what fraction of the reserve each reward will take up, the POC uses the initial values forlastStartDrawFractionandlastFinishDrawFraction, which are likely to increase in production:This coded POC illustrates a real scenario that is likely-to-occur, calculating the fraction of the reserve that the start and finish auctions will need to use. As it can be seen, the fractions become more than 100% of the reserve, which will later cause a revert. The two functions can be pasted in any testing file, as long as the required imports are implemented. The
fractionalRewardfunction is just a copy of theRewardLib.fractionalRewardfunction.As a disclaimer, this is not the only scenario where the issue occurs, there are several other likely-to-occur combinations of
lastStartDrawFraction,lastFinishDrawFraction, number of auctions and elapsed auction times, which will cause an underflow of the reserve. For instance, in the POC above if there are two start auctions both elapsing 5 hours, the fractions would have outgrown the reserve, even before adding the finish draw auction.Impact
Many draws will fail to be awarded.
Code Snippet
https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-draw-manager/src/DrawManager.sol#L334-L352
Tool used
Manual Review
Recommendation
There are several ways to resolve the issue. One solution would be to add a maximum value of the fractions of the reserve that each auction may utilize for rewards. For example, if there are two start auctions and one finish auction, each auction reward is limitied to a third of the reserve.
Duplicate of #125