Loss of funds when buying tickets with no frontend

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L110-L131

Vulnerability details

When tickets are bought, the protocol allows to specify a frontend that will receive a percentage of the ticket fee as rewards (10% for the current setup). However, if this input is left empty during purchase, frontend rewards will still be counted and associated with the zero address.

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L110-L131

function buyTickets(
    uint128[] calldata drawIds,
    uint120[] calldata tickets,
    address frontend,
    address referrer
)
    external
    override
    requireJackpotInitialized
    returns (uint256[] memory ticketIds)
{
    if (drawIds.length != tickets.length) {
        revert DrawsAndTicketsLenMismatch(drawIds.length, tickets.length);
    }
    ticketIds = new uint256[](tickets.length);
    for (uint256 i = 0; i < drawIds.length; ++i) {
        ticketIds[i] = registerTicket(drawIds[i], tickets[i], frontend, referrer);
    }
    referralRegisterTickets(currentDraw, referrer, msg.sender, tickets.length);
    frontendDueTicketSales[frontend] += tickets.length;
    rewardToken.safeTransferFrom(msg.sender, address(this), ticketPrice * tickets.length);
}

Note: protocol owners have confirmed over Discord messaging that frontend is optional and sending the zero address is a valid case.

Impact

In this scenario, frontend rewards will be lost as these will be associated with the zero address. These rewards won't be claimable since the claimRewards function will only transfer rewards associated to the caller.

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L151-L157

function claimRewards(LotteryRewardType rewardType) external override returns (uint256 claimedAmount) {
    address beneficiary = (rewardType == LotteryRewardType.FRONTEND) ? msg.sender : stakingRewardRecipient;
    claimedAmount = LotteryMath.calculateRewards(ticketPrice, dueTicketsSoldAndReset(beneficiary), rewardType);

    emit ClaimedRewards(beneficiary, claimedAmount, rewardType);
    rewardToken.safeTransfer(beneficiary, claimedAmount);
}

Proof of Concept

In the following test, a user specifies address(0) as the frontend while purchasing tickets. The call succeeds and we can query from the address(0) to show the lost rewards:

Note: buyTickets helper has been modified to allow setting an arbitrary frontend address.

contract AuditTest is LotteryTestBase {
    function test_Lottery_buyTickets_EmptyFrontend() public {
        address user = makeAddr("user");
        address frontend = address(0);

        vm.prank(user);
        buySameTickets(lottery.currentDraw(), uint120(0x0F), address(0), frontend, 1);

        // prank address(0) just to query the lost rewards
        vm.prank(frontend);
        uint256 lostRewards = lottery.unclaimedRewards(LotteryRewardType.FRONTEND);
        assertTrue(lostRewards > 0);
    }
}

Recommendation

If frontend is optional, then this case should be specially treated when the zero address is sent as the frontend parameter. Either associated them to staking or to protocol profits.

[c4-judge]

thereksfour marked the issue as duplicate of #483

[thereksfour]

https://github.com/code-423n4/org/issues/53. Require user mistakes, QA. Submit QA as High, invalid

[c4-judge]

thereksfour marked the issue as not a duplicate

[c4-judge]

thereksfour marked the issue as unsatisfactory: Overinflated severity