almurhasan - minTicketsThreshold check inconsistency in function _checkShouldDraw and function _checkShouldCancel.

[sherlock-admin3]

almurhasan

Medium

minTicketsThreshold check inconsistency in function _checkShouldDraw and function _checkShouldCancel.

Summary

currentTicketSold must be equal to/greater than minTicketsThreshold in function _checkShouldDraw but in function _checkShouldCancel, currentTicketSold must be equal to/less than minTicketsThreshold.so if currentTicketSold is reached to/equal to minTicketsThreshold, then cancelRaffle is still possible which is unfair.

Vulnerability Detail

1. Let’s assume, the current raffleId is 1 and the raffleId’s minTicketsThreshold = 100.

2. Now raffleId 1’s 100 tickets are sold. So currentTicketSold = 100.

3. After That, no one buys raffleId 1’s tickets(i.e currentTicketSold is still 100) and raffle.endsAt is also reached(i.e current time is bigger than raffle.endsAt).

4. Now function drawWinner is called which calls function _checkShouldDraw, see function _checkShouldDraw, as currentTicketSold is equal to minTicketsThreshold and current time is bigger than raffle.endsAt, so function drawWinner will be executed.

5. If before calling function drawWinner, the function cancelRaffle is called which calls function _checkShouldCancel, the function cancelRaffle will be executed even though raffleId’s minTicketsThreshold is reached.

Impact

Attackers can frontrun the function cancelRaffle before function drawWinner when currentTicketSold is equal to minTicketsThreshold and current time is bigger than raffle.endsAt.

Code Snippet

https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesTicketManager.sol#L440

Tool used

Manual Review

Recommendation

Instead of if (supply > raffle.minTicketsThreshold) revert TargetTicketsReached();
Put this, if (supply >= raffle.minTicketsThreshold) revert TargetTicketsReached(); in function _checkShouldCancel.

Duplicate of #26

[Almur100]

This is duplicate of #26

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
Winnables/public-contracts#23