denzi_ - `ERC20Incentive::drawRaffle()` does not set limit = 0 allowing further calls to claim to be accepted after the raffle is over.

[sherlock-admin2]

denzi_

High

ERC20Incentive::drawRaffle() does not set limit = 0 allowing further calls to claim to be accepted after the raffle is over.

Summary

ERC20Incentive has two strategies, Raffles and Pools. This submission highlights a fault in the drawRaffle() function where limit is not reset to 0 so that further calls to get an entry in to the raffle are reverted and claimFee is not accepted from such calls.

The incentive owner can create raffle for users to join. Users join the raffle by calling the claim function through the claimIncentive() function provided in BoostCore.sol. This functions takes a small fee of 0.000075 ether from the user.

Vulnerability Detail

The issues lies in the following drawRaffle() function

function drawRaffle() external override onlyOwner {
        if (strategy != Strategy.RAFFLE) revert BoostError.Unauthorized();

        LibPRNG.PRNG memory _prng = LibPRNG.PRNG({state: block.prevrandao + block.timestamp});

        address winnerAddress = entries[_prng.next() % entries.length];

        asset.safeTransfer(winnerAddress, reward);
        emit Claimed(winnerAddress, abi.encodePacked(asset, winnerAddress, reward));
    }

The function checks if the strategy is set to Raffle and then picks out the winner address to which the raffle amount is sent to. The issue is that limit is not set to 0.

limit is used to determine the total number of tickets/entries that can be made. The claim() function which is used to enter into the raffle first checks the limit by calling _isClaimable() function.

function claim(address claimTarget, bytes calldata) external override onlyOwner returns (bool) {
        if (!_isClaimable(claimTarget)) revert NotClaimable();

        // more code below

The isClaimable() function contains the following check

function _isClaimable(address recipient_) internal view returns (bool) {
        return !claimed[recipient_] && claims < limit;
    }

if the current number of claims is less than the limit, the claim() functions proceeds otherwise it will revert. Now back to the issue, if limit is set to 0. Here the code will revert and not allow further claim() calls to proceed indicating that the raffle is over.

Consider the following situation

Bob has created a raffle which is very popular with a high amount of reward and a lot of entries are being made. A few users transactions are pending aswell.

Bob decides to draw the raffle to select the winner out of all the current entries. So Bob calls drawRaffle(). The function gets executed before all the pending calls to claim(). The winner is sent the reward but the raffle does not set the limit to 0, which causes the pending calls of the users to claim() execute aswell. This causes users to enter into the raffle which has already ended and each user loses the claimFee amount required to execute the claim.

This is very equivalent to slippage protection seen commonly, here the user's require protection so that their calls do not execute once the raffle is over.

Impact

User's lose claimFee and enter into a raffle which is now over with no reward in it.

Code Snippet

drawRaffle()

Tool used

Manual Review

Recommendation

function drawRaffle() external override onlyOwner {
        if (strategy != Strategy.RAFFLE) revert BoostError.Unauthorized();

        LibPRNG.PRNG memory _prng = LibPRNG.PRNG({state: block.prevrandao + block.timestamp});

        address winnerAddress = entries[_prng.next() % entries.length];

+       limit = 0;
        asset.safeTransfer(winnerAddress, reward);
        emit Claimed(winnerAddress, abi.encodePacked(asset, winnerAddress, reward));
    }