A lottery winner can steal rewards from other winners and drain the ThrusterTreasure's funds.

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-02-thruster/blob/3896779349f90a44b46f2646094cb34fffd7f66e/thruster-protocol/thruster-treasure/contracts/ThrusterTreasure.sol#L128-L131

Vulnerability details

Impact

A prize can have multiple winners by design. However, one winner will steal rewards from other winners and the funds of ThrusterTreasure.sol can be drained.

Proof of Concept

In ThrusterTreasure.sol, One prize can be set with with multiple winners and this is implemented through setWinningTickets() and setPrize() where _numWinners are set by owner and checked to match the length of winning tickets.

The problem is all the funds for a prize will be distributed to the first winner who calls claimPrizesForRound(), stealing funds from other winners. And if there are still funds left in the contract, the subsequent claimers will steal funds from other prizes.

The vulnerability is current implementation lets a winner get the total amount of the prize instead of prize/_numWinners.

//thruster-protocol/thruster-treasure/contracts/ThrusterTreasure.sol
    function _claimPrize(
        Prize memory _prize,
        address _receiver,
        uint256 _winningTicket
    ) internal {
        uint256 amountETH = _prize.amountWETH;
        uint256 amountUSDB = _prize.amountUSDB;
        //@audit this transfer the total amount of prize to one winner instead of dividing it by the numWinners
|>      WETH.transfer(_receiver, amountETH);
|>      USDB.transfer(_receiver, amountUSDB);
        emit ClaimedPrize(
            _receiver,
            _prize.round,
            _prize.prizeIndex,
            amountETH,
            amountUSDB,
            _winningTicket
        );

(https://github.com/code-423n4/2024-02-thruster/blob/3896779349f90a44b46f2646094cb34fffd7f66e/thruster-protocol/thruster-treasure/contracts/ThrusterTreasure.sol#L128-L131)

For reference, when the owner set the prize, the _prize.amountWETH and _prize.amountUSDB are intended to be shared amoung _numWinners. And _prize.amountWETH and _prize.amountUSDB are transferred as total prize amount.

//thruster-protocol/thruster-treasure/contracts/ThrusterTreasure.sol
    function setPrize(
        uint256 _round,
        uint64 _prizeIndex,
        uint256 _amountWETH,
        uint256 _amountUSDB,
        uint64 _numWinners
    ) external onlyOwner {
        require(_round >= currentRound, "ICR");
        require(_prizeIndex < maxPrizeCount, "IPC");
        //@audit-info note: the amount of _amountWETH, and _amountUSDB are transferred as the total prize amount to be shared. 
|>      depositPrize(msg.sender, _amountWETH, _amountUSDB);
        prizes[_round][_prizeIndex] = Prize(
            _amountWETH,
            _amountUSDB,
            _numWinners,
            _prizeIndex,
            uint64(_round)
        );
    }

    function depositPrize(
        address _from,
        uint256 _amountWETH,
        uint256 _amountUSDB
    ) internal {
        WETH.transferFrom(_from, address(this), _amountWETH);
        USDB.transferFrom(_from, address(this), _amountUSDB);
        emit DepositedPrizes(_amountWETH, _amountUSDB);
    }

(https://github.com/code-423n4/2024-02-thruster/blob/3896779349f90a44b46f2646094cb34fffd7f66e/thruster-protocol/thruster-treasure/contracts/ThrusterTreasure.sol#L169)

As a result, claimPrizesForRound() which wraps _claimPrize() will always send the entire prize amount to the first winner who calls. Subsequent callers(winners) will either have nothing to claim if the contract is already drained or continuously steal funds set aside for other winners/prizes.

Tools Used

Manual

Recommended Mitigation Steps

Factor in _numWinners in _claimPrize().

Assessed type

Math

[jooleseth]

27 is the intended implementation, as the prize amount is the amount set, it is our mistake for not multiplying by the number of winners, however this does not break the functionality, as the admin can still directly send prize funds to the contract without needing to call a specific method, as prizes are taken from the contract's balance. And I agree with #27 analysis of the issue as Medium

[c4-judge]

0xleastwood changed the severity to 2 (Med Risk)

[c4-judge]

0xleastwood marked the issue as duplicate of #27

[c4-judge]

0xleastwood marked the issue as satisfactory