Lucene search

K
code423n4Code4renaCODE423N4:2023-03-WENWIN-FINDINGS-ISSUES-385
HistoryMar 09, 2023 - 12:00 a.m.

Wrong calculation in calculateNewProfit

2023-03-0900:00:00
Code4rena
github.com
5
vulnerability impact
proof of concept
mitigation steps .

Lines of code
<https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L240&gt;
<https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L209&gt;
<https://github.com/code-423n4/2023-03-wenwin/blob/main/src/Lottery.sol#L212&gt;

Vulnerability details

Impact

There is a wrong calculation of the cumulative net profit of the lottery, which affects the calculation of the excess pot and rewards per winning ticket (including the jackpot) in each draw. This vulnerability also leads to a Denial of Service of the Lottery contract.

Proof of Concept

The result of LotteryMath.calculateMultiplier should be used to calculate a percentage of the expected payout for all tickets. This should be done using the getPercentage method from the PercentageMath library, as is done in LotteryMath.calculateReward:

  96:    function calculateReward(
            ...
 107:    {
 108:        uint256 excess = calculateExcessPot(netProfit, fixedJackpot);
 109:        reward = isJackpot
 110:            ? fixedReward + excess.getPercentage(EXCESS_BONUS_ALLOCATION) // @audit &lt;- PoC
 111:            : fixedReward.getPercentage(calculateMultiplier(excess, ticketsSold, expectedPayout));
 112:    }

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

The problem is that instead of calculating the expectedRewardsOut in LotteryMath.calculateNewProfit as a percentage from ticketsSold * expectedPayout, the current implementation will multiply the result of the multiplication by the returned basis points from calculateMultiplier. This will result in making the newProfit extremely high and therefore impact the calculation of rewards and excess pot in all future draws. This can also lead to a DoS because the calculated profit will always be extremely lowered each time the jackpot is not won (since expectedRewardsOut will grow exponentially due to the excess pot, and the multiplier will grow as well) and therefore come to an underflow situation after a certain time has passed.

Tools Used

Manual Review

Recommended Mitigation Steps

Fix the code in LotteryMath.calculateNewProfit in the following way:

  50:    uint256 expectedRewardsOut = jackpotWon
  51:        ? calculateReward(oldProfit, fixedJackpotSize, fixedJackpotSize, ticketsSold, true, expectedPayout)
- 52:        : calculateMultiplier(calculateExcessPot(oldProfit, fixedJackpotSize), ticketsSold, expectedPayout)
- 53:            * ticketsSold * expectedPayout;
+ 52:        : (ticketsSold * expectedPayout).getPercentage(
+ 53:           calculateMultiplier(
+ 54:               calculateExcessPot(oldProfit, fixedJackpotSize),
+ 55:               ticketsSold,
+ 56:               expectedPayout
+ 57:           )
+ 58:       );

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


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

All reactions