jsmi - Attacker can cancel raffle when there is exactly minimum participants.

[sherlock-admin2]

jsmi

Medium

Attacker can cancel raffle when there is exactly minimum participants.

Summary

When there is exactly minimum participants, both drawing winner and cancelling raffle are available at the same time.
Attacker can exploit this vulnerability and cancel the raffle.

Vulnerability Detail

When tickets sale period finishes, anyone can call WinnablesTicketManager.drawWinner() which in turn calls the following _checkShouldDraw() to validate some conditions.

    function _checkShouldDraw(uint256 raffleId) internal view {
        Raffle storage raffle = _raffles[raffleId];
        if (raffle.status != RaffleStatus.IDLE) revert InvalidRaffle();
        uint256 currentTicketSold = IWinnablesTicket(TICKETS_CONTRACT).supplyOf(raffleId);
        if (currentTicketSold == 0) revert NoParticipants();

        if (block.timestamp < raffle.endsAt) {
            if (currentTicketSold < raffle.maxTicketSupply) revert RaffleIsStillOpen();
        }
431     if (currentTicketSold < raffle.minTicketsThreshold) revert TargetTicketsNotReached();
    }

From L431, the sold ticket count should be equal or larger than minimum threshold to start drawing winner.
On the other hand, attacker can call WinnablesTicketManager.cancelRaffle() which in turn calls the following _checkShouldCancel().

    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);
440     if (supply > raffle.minTicketsThreshold) revert TargetTicketsReached();
    }

From L440, the sold ticket count should be less or equal than minimum threshold to cancel raffle.
As a result, when the sold ticket count is equal to minimum threshold, both drawing winner and canceling raffle are available.
Attacker can cancel the raffle before any other users drawing winner in such a case

Impact

Attacker can cancel raffle when there is exactly minimum participants and this means DoS of core function of contract.

Code Snippet

• WinnablesTicketManager._checkShouldDraw()
• WinnablesTicketManager._checkShouldCancel()

Tool used

Manual Review

Recommendation

Modify WinnablesTicketManager._checkShouldDraw() function as follows.

    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();
    }

Duplicate of #26

[WangSecurity]

This will be duplicated with #26 and validated with Medium severity.

[sherlock-admin2]

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