sherlock-audit / 2024-05-pooltogether-judging

2 stars 0 forks source link

cu5t0mPe0 - After the user calls startDraw, they are unable to obtain the reward. #150

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

cu5t0mPe0

medium

After the user calls startDraw, they are unable to obtain the reward.

Summary

After calling finishDraw, it is still possible to call startDraw, causing the user to be unable to obtain the reward.

Vulnerability Detail

startDraw does not check whether finishDraw has already been called. Therefore, if a user calls startDraw after calling finishDraw, the user will be unable to obtain the deserved reward.

Additionally, if the user tries to call finishDraw again, it will fail because after the first call to finishDraw, awardDraw sets the _lastAwardedDrawId in the prizePool to the return value of prizePool.getDrawIdToAward.

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/abstract/TieredLiquidityDistributor.sol#L249

When called the second time, _lastAwardedDrawId will be equal to the current drawId.

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/PrizePool.sol#L460

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/PrizePool.sol#L467

Then, when calling __awardDraw, it first calls getTotalContributedBetween.

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/PrizePool.sol#L479

getTotalContributedBetween calls getDisbursedBetween. Since the parameters passed before calling getTotalContributedBetween will set lastAwardedDrawId_ + 1, it will trigger the following condition.

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/libraries/DrawAccumulatorLib.sol#L147-L148

Therefore, if an attacker calls finishDraw before a user calls startDraw, the user will no longer be able to call the finishDraw function, resulting in the loss of their deserved reward amount.

For example:

After the attacker calls startDraw, they notice that User A is about to call startDraw. The attacker then front-runs the transaction and executes finishDraw first. At this point, User A thinks that after executing startDraw, they will receive a reward. However, when User A calls finishDraw, it will revert, causing User A to lose their reward.

Impact

The user will lose the reward fee they deserve

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-draw-manager/src/DrawManager.sol#L219-L273

Tool used

Manual Review

Recommendation

It should check whether the drawId has already completed the finishDraw operation.

sherlock-admin2 commented 1 month ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

startDraw cannot be called again if the RNG request hasn't failed

nevillehuang commented 1 month ago

Invalid, sponsor comments:

  • invalid, startDraw checks to ensure that the draw ID to award has closed. Since finishDraw awards the draw, this will increase the drawIdToAward by 1 and startDraw will fail until the next draw closes