0rpse - Link token can be used as prize to prevent users from getting their prize

[sherlock-admin3]

0rpse

High

Link token can be used as prize to prevent users from getting their prize

Summary

Using Link token as prize can cause underflow and break internal accounting, letting admins have the funds from ticket sales and withdraw locked link but winning player will not be able to get the prize.

Vulnerability Detail

Whenever a Token is locked as prize a mapping is updated for how many locked tokens are in the contract, this is used to determine how many tokens admin can withdraw from the contract.

    function lockTokens(
        address ticketManager,
        uint64 chainSelector,
        uint256 raffleId,
        address token,
        uint256 amount
    ) external onlyRole(0) {
        RafflePrize storage rafflePrize = _checkValidRaffle(raffleId);
        uint256 tokenBalance = IERC20(token).balanceOf(address(this));
        if (tokenBalance < amount + _tokensLocked[token]) revert InvalidPrize();
        rafflePrize.raffleType = RaffleType.TOKEN;
        unchecked { _tokensLocked[token] += amount; } //@audit keep track of locked tokens
        _tokenRaffles[raffleId].tokenAddress = token;
        _tokenRaffles[raffleId].amount = amount;

        _sendCCIPMessage(ticketManager, chainSelector, abi.encodePacked(raffleId));
        emit TokenPrizeLocked(raffleId, token, amount);
    }

Because the contract spends Link token by using CCIP, tokenBalance will decrease and the unchecked line in withdrawToken can underflow. This means admin can withdraw any Link tokens even if they are locked.

    function withdrawToken(address token, uint256 amount) external onlyRole(0) {
        uint256 tokenBalance = IERC20(token).balanceOf(address(this));
        uint256 availableBalance;
        unchecked { availableBalance = tokenBalance - _tokensLocked[token]; }
        if (availableBalance < amount) revert InsufficientBalance();
        IERC20(token).safeTransfer(msg.sender, amount);
    }

Moreover, after the raffle concludes, winner will not be able to withdraw the prize Link tokens as the raffle amount will not be present on the contract due to CCIP usage or admin withdrawing tokens and the transfer will revert.

    function claimPrize(uint256 raffleId) external {
        RafflePrize storage rafflePrize = _rafflePrize[raffleId];
        RaffleType raffleType = rafflePrize.raffleType;
        //---omitted
        } else if (raffleType == RaffleType.TOKEN) {
            TokenInfo storage raffle = _tokenRaffles[raffleId];
            unchecked { _tokensLocked[raffle.tokenAddress] -= raffle.amount; }
            _sendTokenPrize(raffle.tokenAddress, raffle.amount, msg.sender);
       // ---omitted---        
    }

    function _sendTokenPrize(address token, uint256 amount, address winner) internal {
        IERC20(token).safeTransfer(winner, amount);
    }

Impact

Admin can take users funds gathered from ticket sales all the while keeping enough Link tokens in the contract so it keeps functioning, winner's prize will be locked.

Code Snippet

https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/81b28633d0f450e33a8b32976e17122418f5d47e/public-contracts/contracts/WinnablesPrizeManager.sol#L196-L213 https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/81b28633d0f450e33a8b32976e17122418f5d47e/public-contracts/contracts/WinnablesPrizeManager.sol#L218-L224 https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/81b28633d0f450e33a8b32976e17122418f5d47e/public-contracts/contracts/WinnablesPrizeManager.sol#L105-L124 https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/81b28633d0f450e33a8b32976e17122418f5d47e/public-contracts/contracts/WinnablesPrizeManager.sol#L308

Tool used

Manual Review

Recommendation

Disallow using Link token as prize.

Duplicate of #277

[0rpse]

Escalate As link tokens are valid ERC20 tokens, they can be used by the admin to create raffles and damage protocol users. Also considering these statements from the contest README:

The principles that must always remain true are:

Winnables admins cannot do anything to prevent a winner from withdrawing their prize Participants in a raffle that got cancelled can always get refunded Admins cannot affect the odds of a raffle

this issue should be a valid high.

Also note that it does not break these statements:

The following assumptions are to be made about the admin’s behavior without being enforced:

Admins will not abuse signatures to get free tickets or to distribute them to addresses they control. Admins will always keep LINK in the contracts (to pay for CCIP Measages) and in the VRF subscription (to pay for the random numbers)

Admin can keep LINK in the contract but the winner won't be able to withdraw any, and admin has the ability to withdraw all.

[sherlock-admin3]

Escalate As link tokens are valid ERC20 tokens, they can be used by the admin to create raffles and damage protocol users. Also considering these statements from the contest README:

The principles that must always remain true are:

Winnables admins cannot do anything to prevent a winner from withdrawing their prize Participants in a raffle that got cancelled can always get refunded Admins cannot affect the odds of a raffle

this issue should be a valid high.

Also note that it does not break these statements:

The following assumptions are to be made about the admin’s behavior without being enforced:

Admins will not abuse signatures to get free tickets or to distribute them to addresses they control. Admins will always keep LINK in the contracts (to pay for CCIP Measages) and in the VRF subscription (to pay for the random numbers)

Admin can keep LINK in the contract but the winner won't be able to withdraw any, and admin has the ability to withdraw all.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[mystery0x]

All admin related issues have been discussed with the HOJ and the sponsor who wished the following governing statements could have been more carefully written and/or omitted:

"The protocol working as expected relies on having an admin creating raffles. It should be expected that the admin will do their job. However it is not expected that the admin can steal funds that should have ended in a raffle participant’s wallet in any conceivable way."

Much as many of the findings suggest good fixes, the conclusion is that they will all be deemed low unless it's beyond the trusted admin control.

Also there’s this line: “Admins will always keep LINK in the contracts (to pay for CCIP Measages) and in the VRF subscription (to pay for the random numbers)” in the README, so in this case having not enough LINK would be against this line and a mistake.

[0rpse]

I already pointed out that the issue does not go against "Admins will always keep LINK in the contracts (to pay for CCIP Measages) and in the VRF subscription (to pay for the random numbers)", only provided that admin can withdraw all LINK even though there are checks against this as further analysis. It's perfectly possible to still keep all LINK in the contract and the winner will not be able to withdraw any.

Also note even if admin is always acting in good faith, this issue can occur in natural progression as LINK token is valid ERC20.

[Brivan-26]

Using Link token as prize can cause underflow and break internal accounting, letting admins have the funds from ticket sales and withdraw locked link but winning player will not be able to get the prize.

This falls under Admin trust assumption, if he locks LINK as a prize, he needs to provide surplus amount than the one allocated for Chainlink transactions

[WangSecurity]

After additionally considering this issue, the escalation will be accepted and the issue will be validated with medium severity and duplicated with #277. For more details, look into the discussion under #277.

[WangSecurity]

Result: Medium Duplicate of #277

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• 0rpse: accepted

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/Winnables/public-contracts/pull/18