Lines of code

Vulnerability details

The function calculateNewProfit present in the LotteryMath library is used when finalizing the current draw in the Lottery to track and update the currentNetProfit variable in the contract.

<https://github.com/code-423n4/2023-03-wenwin/blob/main/src/LotteryMath.sol#L35-L56&gt;

function calculateNewProfit(
    int256 oldProfit,
    uint256 ticketsSold,
    uint256 ticketPrice,
    bool jackpotWon,
    uint256 fixedJackpotSize,
    uint256 expectedPayout
)
    internal
    pure
    returns (int256 newProfit)
{
    uint256 ticketsSalesToPot = (ticketsSold * ticketPrice).getPercentage(TICKET_PRICE_TO_POT);
    newProfit = oldProfit + int256(ticketsSalesToPot);

    uint256 expectedRewardsOut = jackpotWon
        ? calculateReward(oldProfit, fixedJackpotSize, fixedJackpotSize, ticketsSold, true, expectedPayout)
        : calculateMultiplier(calculateExcessPot(oldProfit, fixedJackpotSize), ticketsSold, expectedPayout)
            * ticketsSold * expectedPayout;

    newProfit -= int256(expectedRewardsOut);
}

When jackpotWon == false, the function calls calculateMultiplier, which returns a scaled percentage, and misses to call getPercentage to scale down the value.

Impact

In the case that the jackpot isn’t won in the current draw, the profit calculation will be wrongly scaled by a factor of PERCENTAGE_BASE (100_000) and will result in higher values that are offsetted by several orders of magnitude.

This error will cascade into rewards, as the currentNetProfit variable is used in the drawRewardSize function, which is used to calculate reward values and bonuses:

<https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L238-L247&gt;

function drawRewardSize(uint128 drawId, uint8 winTier) private view returns (uint256 rewardSize) {
    return LotteryMath.calculateReward(
        currentNetProfit,
        fixedReward(winTier),
        fixedReward(selectionSize),
        ticketsSold[drawId],
        winTier == selectionSize,
        expectedPayout
    );
}

PoC

In the following test, a user purchases just 10 tickets that aren’t the winning ticket (meaning no jackpot is won). The updated
net profit ends up being -379965 DAI!

contract AuditTest is LotteryTestBase {
  function test_LotteryMath_calculateNewProfit_WrongCalculationWhenJackpotNotWon() public {
        // At start current profit is 0
        int256 currentProfit = lottery.currentNetProfit();
        assertEq(currentProfit, 0);

        // User buys some tickets
        address user = makeAddr("user");
        vm.startPrank(user);
        buySameTickets(lottery.currentDraw(), uint120(0x0F), address(0), 10);
        vm.stopPrank();

        // Finalize draw with no jackpot
        finalizeDraw(0x01020304);

        // Net profit is now -379965 * (10 ** 18)!
        currentProfit = lottery.currentNetProfit();
        assertEq(currentProfit, -379_965 ether);
    }
}

Recommendation

The calculation should use getPercentage to correctly scale down the percentage value:

function calculateNewProfit(
    int256 oldProfit,
    uint256 ticketsSold,
    uint256 ticketPrice,
    bool jackpotWon,
    uint256 fixedJackpotSize,
    uint256 expectedPayout
)
    internal
    pure
    returns (int256 newProfit)
{
    uint256 ticketsSalesToPot = (ticketsSold * ticketPrice).getPercentage(TICKET_PRICE_TO_POT);
    newProfit = oldProfit + int256(ticketsSalesToPot);

    uint256 expectedRewardsOut = jackpotWon
        ? calculateReward(oldProfit, fixedJackpotSize, fixedJackpotSize, ticketsSold, true, expectedPayout)
        : (ticketsSold * expectedPayout).getPercentage(calculateMultiplier(calculateExcessPot(oldProfit, fixedJackpotSize), ticketsSold, expectedPayout));

    newProfit -= int256(expectedRewardsOut);
}  

---

The text was updated successfully, but these errors were encountered:

All reactions