shikhar - _checkShouldCancel doesn't revert for the case when tickets sold are equal to minimum ticket threshold due to insufficient check.

[sherlock-admin4]

shikhar

Medium

_checkShouldCancel doesn't revert for the case when tickets sold are equal to minimum ticket threshold due to insufficient check.

Summary

• The _checkShouldCancel simply returns to the caller function if Raffle is to be cancelled otherwise it reverts.
• It was expected that it should also revert, if tickets sold are exactly equal to minimum threshold or more than threshold.
• But it doesn't revert when tickets sold are exactly equal to minimum ticket threshold due to incorrect check.
• As a result of which even if the supply reaches minimum ticket threshold, the Raffle is cancellable and a malicious user can call cancelRaffle function. Thus, leaving an eligible Raffle into a cancelled Raffle and the lucky participant will not get the prize.

Vulnerability Detail

• The vulnerability is present in the _checkShouldCancel function where it doesn't revert for the case when supply is exactly minimum ticket threshold (when Raffle end time has reached).
• This occurs due to the below check (represented by @>). Here, it only reverts if supply is more than minimum threshold, but it is also expected to revert when supply is exactly minimum tickets threshold.

    function _checkShouldCancel(uint256 raffleId) internal view {
        Raffle storage raffle = _raffles[raffleId];
        if (raffle.status == RaffleStatus.PRIZE_LOCKED) return;
        if (raffle.status != RaffleStatus.IDLE) revert InvalidRaffle();
        if (raffle.endsAt > block.timestamp) revert RaffleIsStillOpen();
        uint256 supply = IWinnablesTicket(TICKETS_CONTRACT).supplyOf(raffleId);
@>      if (supply > raffle.minTicketsThreshold) revert TargetTicketsReached();
    }

• As a result of this incorrect check, it doesn't revert and simple returns which means that the calling function can continue with cancellation for the above discussed case.
• Thus, resulting in a cancellation of Raffle which was eligible of drawing a winner and would be left without a winner.

Impact

• Raffles can be cancelled by malicious user for the case when supply is exactly minimum tickets thresholds, resulting in Raffle cancellation and no winner chosen.

Code Snippet

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

Tool used

Manual Review

Recommendation

Correct the check as below:

    function _checkShouldCancel(uint256 raffleId) internal view {
        Raffle storage raffle = _raffles[raffleId];
        if (raffle.status == RaffleStatus.PRIZE_LOCKED) return;
        if (raffle.status != RaffleStatus.IDLE) revert InvalidRaffle();
        if (raffle.endsAt > block.timestamp) revert RaffleIsStillOpen();
        uint256 supply = IWinnablesTicket(TICKETS_CONTRACT).supplyOf(raffleId);
-       if (supply > raffle.minTicketsThreshold) revert TargetTicketsReached();
+       if (supply >= raffle.minTicketsThreshold) revert TargetTicketsReached();
    }

[shikhar229169]

This finding is a dup of #26

[sherlock-admin2]

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