WinnablesTicketManager.sol#L347-L361

Summary

The implementation of WinnablesTicketManager::fulfillRandomWords function is not as per the guidelines provided by Chainlink VRF.

Vulnerability Detail

The WinnablesTicketManager::fulfillRandomWords function:

    /// @notice (Chainlink VRF Coordinator) Use given random number as a result to determine the winner of a Raffle
    /// @param requestId ID of the VRF request to fulfill
    /// @param randomWords Array of 32 bytes integers sent back from the oracle
    function fulfillRandomWords(
        uint256 requestId,
        uint256[] memory randomWords
    ) internal override {
        RequestStatus storage request = _chainlinkRequests[requestId];
        Raffle storage raffle = _raffles[request.raffleId];
        if (raffle.status != RaffleStatus.REQUESTED) revert RequestNotFound(requestId);
        //@audit as per chainlink VRF documentation the function must not revert 
        // but the above statement in case the status is not "REQUESTED" then it will revert
        // https://docs.chain.link/vrf/v2/security#fulfillrandomwords-must-not-revert
        request.randomWord = randomWords[0];
        raffle.status = RaffleStatus.FULFILLED;
        emit WinnerDrawn(requestId);
        IWinnablesTicket(TICKETS_CONTRACT).refreshMetadata(request.raffleId);
    }

As per the VRF Security Considerations : fulfillRandomWords must not revert . Check here

But in the function this line :

 if (raffle.status != RaffleStatus.REQUESTED) revert RequestNotFound(requestId);

In case the if statement returns true, which will occur in the case when raffle.status != RaffleStatus.REQUESTED then the function wil revert. Which is not desireable according to the guidelines by Chainlink VRF.

Impact

The function is not as per guidelines provided by chainlink can give undesirable results.

Code Snippet

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

Tool used

Manual Review

Recommendation

Update the logic such so that there is no revert statement present in the function and follow the guidelines by Chainlink.

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

tdey - https://github.com/sherlock-audit/2024-08-winnables-raffles-tamoghna-dey/blob/81b28633d0f450e33a8b32976e17122418f5d47e/public-contracts/contracts/WinnablesTicketManager.sol#L347-L361 #554