sherlock-audit / 2024-08-winnables-raffles-judging

6 stars 2 forks source link

denzi_ - Denial of Service Vulnerability in Raffle Cancellation Logic #599

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

denzi_

Medium

Denial of Service Vulnerability in Raffle Cancellation Logic

Summary

A denial of service (DoS) vulnerability exists in the cancelRaffle function of the WinnablesTicketManager contract. This flaw allows any user to prematurely cancel a raffle by manipulating the raffle's state from PRIZE_LOCKED to CANCELED, thus preventing the proper initiation of the raffle setup process which depends on the raffle being in the PRIZE_LOCKED state.

Vulnerability Detail

The WinnablesPrizeManager.sol has three lock functions to lock the rewards i.e. ETH, NFT or TOKENS. Once this function is executing it sends a ccipMessage to the WinnablesTicketManager.sol . The purpose of this message is to inform the contract that a new prize has been locked and a raffle should be created for it.

Upon receiving this message the _ccipReceive() function sets the status of the raffle through

_raffles[raffleId].status = RaffleStatus.PRIZE_LOCKED;

The status is updated and prize is locked. After this createRaffle() has to be called by the admin to create a raffle with the right parameters so that the raffle can start but there is another function in the contract which allows a malicious user to DoS the creation of new raffles.

    /// @notice (Public) Cancel a raffle if it can be canceled
    /// @param raffleId ID of the raffle to cancel
    function cancelRaffle(address prizeManager, uint64 chainSelector, uint256 raffleId) external {
        _checkShouldCancel(raffleId);

        _raffles[raffleId].status = RaffleStatus.CANCELED;
        _sendCCIPMessage(
            prizeManager,
            chainSelector,
            abi.encodePacked(uint8(CCIPMessageType.RAFFLE_CANCELED), raffleId)
        );
        IWinnablesTicket(TICKETS_CONTRACT).refreshMetadata(raffleId);
    }
function _checkShouldCancel(uint256 raffleId) internal view {
        Raffle storage raffle = _raffles[raffleId];

        // @audit-issue malicious user can dos the contract
        if (raffle.status == RaffleStatus.PRIZE_LOCKED) return; // if prize is still locked then 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(); 
    }

The first function cancelRaffle() calls the internal function _checkShouldCancel function to verify if the raffle can be cancelled. The 2nd function returns after executing

if (raffle.status == RaffleStatus.PRIZE_LOCKED) return;

The flow is not at the first function which will set the new status of the raffle to CANCELED. This will block the raffle from being created on this contract because to create the raffle a check inside createRaffle() function is present i.e.

if (raffle.status != RaffleStatus.PRIZE_LOCKED) revert PrizeNotLocked();

This causes the function to revert and the raffle can not be created.

Impact

DoS of creation of raffles by calling cancelRaffle on the id of the raffle.

Code Snippet

createRaffle() _ccipReceive() cancelRaffle() _checkShouldCancel()

Tool used

Manual Review

Recommendation

Do not allow cancellation of prize locked raffles by any user.

Duplicate of #57