Wenwin contest - bin2chen's results

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/LotteryMath.sol#L52

Vulnerability details

Impact

calculateNewProfit() Missing divide by PERCENTAGE_BASE, resulting in less drawRewardSize() and fewer winTier rewards

Proof of Concept

calculateNewProfit() is used to recalculate the profit, the code is as follows:

    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) //<-----this place miss div PERCENTAGE_BASE
                * ticketsSold * expectedPayout;

        newProfit -= int256(expectedRewardsOut);
    }

expectedRewardsOut is calculated if not a Jackpot, using the following formula calculateMultiplier(calculateExcessPot(oldProfit, fixedJackpotSize), ticketsSold, expectedPayout) * ticketsSold * expectedPayout

calculateMultiplier() returns a precision of 100_000, the above formula is missing the division by PERCENTAGE_BASE, the normal should be to use PercentageMath.getPercentage(calculateMultiplier(calculateExcessPot(oldProfit, fixedJackpotSize), ticketsSold, expectedPayout), ticketsSold * expectedPayout)

This method is used to calculate currentNetProfit, if the number incorrect (negative) resulting in less drawRewardSize() and fewer winTier rewards

Tools Used

Recommended Mitigation Steps

    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;
+            :PercentageMath.getPercentage(calculateMultiplier(calculateExcessPot(oldProfit, fixedJackpotSize),ticketsSold, expectedPayout), 
+             ticketsSold * expectedPayout)`

        newProfit -= int256(expectedRewardsOut);
    }

L-01: Ticket.mint() is recommended to use _safeMint() to avoid the receiver is contract, can not properly handle the NFT, resulting in the loss of NFT

    function mint(address to, uint128 drawId, uint120 combination) internal returns (uint256 ticketId) {
        ticketId = nextTicketId++;
        ticketsInfo[ticketId] = TicketInfo(drawId, combination, false);
-       _mint(to, ticketId);
+       _safeMint(to, ticketId);
    }

L-02:claimable() It is more reasonable to use drawScheduledAt to determine the expiration, currently using ticketRegistrationDeadline () will be less drawCoolDownPeriod, but this is the purchase of cooldown, should not be counted as a draw cycle

    function claimable(uint256 ticketId) external view override returns (uint256 claimableAmount, uint8 winTier) {
        TicketInfo memory ticketInfo = ticketsInfo[ticketId];
        if (!ticketInfo.claimed) {
            uint120 _winningTicket = winningTicket[ticketInfo.drawId];
            winTier = TicketUtils.ticketWinTier(ticketInfo.combination, _winningTicket, selectionSize, selectionMax);
-           if (block.timestamp <= ticketRegistrationDeadline(ticketInfo.drawId + LotteryMath.DRAWS_PER_YEAR)) {            
+           if (block.timestamp <= drawScheduledAt(ticketInfo.drawId + LotteryMath.DRAWS_PER_YEAR)) {
                claimableAmount = winAmount[ticketInfo.drawId][winTier];
            }
        }
    }