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

6 stars 2 forks source link

BlocSoc_Audits - [H-2] Possible Reentrancy in WinnablesPrizeManager::claimPrize() #547

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

BlocSoc_Audits

High

[H-2] Possible Reentrancy in WinnablesPrizeManager::claimPrize()

File: WinnablesPrizeManager

Description:

The function WinnablesPrizeManager::claimPrize() is provided to claim the raffle prize for the winner. In its implementation, The winner can claim the prize of a raffle by providing the raffleId and The asset associated with this raffleId is transferred to the msg.sender address.

However, the checks for the claimed status of the raffle and whether the msg.sender is the winner of the raffle are implemented after the prize has been transferred and the status of the raffle is changed to CLAIMED only after the transfer has taken place. If in an instance, msg.sender is a contract and not an EOA, the malicious actor could trigger reentrancy by repetitively calling the claimPrize() within a receive function and thus draining all the funds since there are no checks for the same before another interaction.

    function claimPrize(uint256 raffleId) external {
        RafflePrize storage rafflePrize = _rafflePrize[raffleId];
        RaffleType raffleType = rafflePrize.raffleType;
        if (raffleType == RaffleType.NFT) {
            NFTInfo storage raffle = _nftRaffles[raffleId];
            _nftLocked[raffle.contractAddress][raffle.tokenId] = false;
            _sendNFTPrize(raffle.contractAddress, raffle.tokenId, msg.sender);
        } else if (raffleType == RaffleType.TOKEN) {
            TokenInfo storage raffle = _tokenRaffles[raffleId];
            unchecked { _tokensLocked[raffle.tokenAddress] -= raffle.amount; }
            _sendTokenPrize(raffle.tokenAddress, raffle.amount, msg.sender);
        } else if (raffleType == RaffleType.ETH) {
            unchecked { _ethLocked -= _ethRaffles[raffleId]; }
            _sendETHPrize(_ethRaffles[raffleId], msg.sender);
        } else revert InvalidRaffle();
        if (msg.sender != rafflePrize.winner) revert UnauthorizedToClaim();
        if (rafflePrize.status == RafflePrizeStatus.CLAIMED) revert AlreadyClaimed();
        rafflePrize.status = RafflePrizeStatus.CLAIMED;
        emit PrizeClaimed(raffleId, msg.sender);
    }

In Reentrancy, the state is updated after the external call and therefore, when reentered, the state is the same as the first call. The caller is a contract with a malicious receive function which calls the vulnerable claimPrize() function and because the checks and the state updation are implemented after the external calls, the funds get transferred again. This again receive for the caller which calls the claimPrize() function with the same state and the funds are drained completely.

Mitigation:

This vulnerability can be easily mitigated by using the checks-effects-interactions practice that helps to prevent reentrancy attacks. The code after this practice has been adhered to is as follows:

    function claimPrize(uint256 raffleId) external {
        RafflePrize storage rafflePrize = _rafflePrize[raffleId];
        RaffleType raffleType = rafflePrize.raffleType;
        // checks 
        if (msg.sender != rafflePrize.winner) revert UnauthorizedToClaim();
        if (rafflePrize.status == RafflePrizeStatus.CLAIMED) revert AlreadyClaimed();
        // effects
        rafflePrize.status = RafflePrizeStatus.CLAIMED;
        emit PrizeClaimed(raffleId, msg.sender);
        if (raffleType == RaffleType.NFT) {
            NFTInfo storage raffle = _nftRaffles[raffleId];
            _nftLocked[raffle.contractAddress][raffle.tokenId] = false;
            // interactions 
            _sendNFTPrize(raffle.contractAddress, raffle.tokenId, msg.sender);
        } else if (raffleType == RaffleType.TOKEN) {
            TokenInfo storage raffle = _tokenRaffles[raffleId];
            unchecked { _tokensLocked[raffle.tokenAddress] -= raffle.amount; }
            _sendTokenPrize(raffle.tokenAddress, raffle.amount, msg.sender);
        } else if (raffleType == RaffleType.ETH) {
            unchecked { _ethLocked -= _ethRaffles[raffleId]; }
            _sendETHPrize(_ethRaffles[raffleId], msg.sender);
        } else revert InvalidRaffle();
    }

Liklihood: High

Impact: HIgh

Tool used

Manual Review