durov - A malicious admin can exploit the system

[sherlock-admin3]

durov

High

A malicious admin can exploit the system

Summary

Malicious admins can steal the prize from any raffle or prevent a raffle from getting canceled, making users' funds stuck

Vulnerability Detail

As the contest's README states,
"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"

However, this is not exactly true, since a malicious admin could approve his own malicious contract using WinnablesPrizeManager.sol::setCCIPCounterpart() function and send forged messages from his contract to the PrizeManager, which could either:

• Steal any locked prize by sending a message which consists of CCIPMessageType.WINNER_DRAWN, raffleId and his address
• Prevent a raffle from getting canceled on Avalanche by sending a forged message which consists of CCIPMessageType.RAFFLE_CANCELED and raffleId, which makes the function cancelRaffle() on Avalanche contract revert and a raffle to get stuck with users' funds.

function _ccipReceive(
        Client.Any2EVMMessage memory message
    ) internal override {
        (address _senderAddress) = abi.decode(message.sender, (address));
        bytes32 counterpart = _packCCIPContract(_senderAddress, message.sourceChainSelector);
        if (!_ccipContracts[counterpart]) revert UnauthorizedCCIPSender();

        CCIPMessageType messageType = CCIPMessageType(uint8(message.data[0]));
        uint256 raffleId;
        address winner;
        if (messageType == CCIPMessageType.RAFFLE_CANCELED) {
            raffleId = _decodeRaffleCanceledMessage(message.data);
            _cancelRaffle(raffleId);
            return;
        }
        (raffleId, winner) = _decodeWinnerDrawnMessage(message.data);
        _rafflePrize[raffleId].winner = winner;
        emit WinnerPropagated(raffleId, winner);
    }

function _cancelRaffle(uint256 raffleId) internal {
        RaffleType raffleType = _rafflePrize[raffleId].raffleType;
        if (_rafflePrize[raffleId].status == RafflePrizeStatus.CANCELED) revert InvalidRaffle();
        . . .
        _rafflePrize[raffleId].status = RafflePrizeStatus.CANCELED;
        emit PrizeUnlocked(raffleId);
    }

function setCCIPCounterpart(
        address contractAddress,
        uint64 chainSelector,
        bool enabled
    ) external onlyRole(0) {
        _setCCIPCounterpart(contractAddress, chainSelector, enabled);
    }

function _setCCIPCounterpart(
        address contractAddress,
        uint64 chainSelector,
        bool enabled
    ) internal {
        bytes32 counterpart = _packCCIPContract(contractAddress, chainSelector);
        _ccipContracts[counterpart] = enabled;
    }

Impact

Malicious admins could abuse the system by stealing prizes or making users' funds stuck in the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesPrizeManager.sol#L260-L278
https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesPrizeManager.sol#L280-L294
https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesPrizeManager.sol#L134-L140
https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/BaseCCIPContract.sol#L31-L38

Tool used

Manual Review

Recommendation

Implement a check that doesn't let admins set multiple CCIPCounterparts since there's only 1 contract on both Ethereum and Avalanche that sends cross-chain messages.

Duplicate of #277

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits:
Winnables/public-contracts#22