code-423n4 / 2023-03-wenwin-findings

1 stars 1 forks source link

Likelihood of becoming zero numbers for the numbers of the winning ticket #487

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/main/src/TicketUtils.sol#L43-L76

Vulnerability details

Impact

The logic of the function reconstructTicket() may give more zero numbers inside the packed winning ticket.

Proof of Concept

As this issue may already be a sponsor-acknowledge case, however, it's worth explaining. Considering the case 7/35 for a lottery game, we need to use the formula for combinations: $$C(n,r) = \binom{n}{k} = \dfrac{n!}{k!(n-k)!} $$ So with the numbers n=35, r=7, we'll get: $$C(35,7) = \binom{35}{7} = \dfrac{35!}{7!(28)! = 35,724,075} $$

For this game, the following prizes are assumed:

Match 7 numbers: Jackpot ($1,800,000) Match 6 numbers: $1,500 Match 5 numbers: $75 Match 4 numbers: $5 Match 3 numbers: $1.5

In order to calculate the RTP of the player, we need to calculate the expected value of each prize, we multiply the probability of winning by the amount of the prize:

Expected value of matching 7 numbers = (1/5,245,786) x $1,800,000 = $0.343 Expected value of matching 6 numbers = (1/26,231) x $1,500 = $0.057 Expected value of matching 5 numbers = (1/517) x $75 = $0.145 Expected value of matching 4 numbers = (1/38) x $5 = $0.132 Expected value of matching 3 numbers = (1/8.6) x $1.5 = $0.174

The total expected value of all possible prizes is the sum of the expected values:

$0.343 + $0.057 + $0.145 + $0.132 + $0.174 = $0.851

Therefore, the RTP is 0.851 or 85.1%.

It sounds great for a game, though there is a small problem. The above-mentioned values are calculated considering the unique random numbers. However, inside the function reconstructTicket(), one random number is selected as the function input. I know this is due to Solidity's and chainlink's restrictions, and we need to pack the number. However, by making the numbers small in each loop, the chance of becoming the zero number of the ticket increases.

        for (uint256 i = 0; i < selectionSize; ++i) {
            numbers[i] = uint8(randomNumber % currentSelectionCount);
            randomNumber /= currentSelectionCount;
            currentSelectionCount--;
        }

In the loop above, if the rest of the randomNumber becomes smaller enough, for some numbers it may be dividable by the currentSelectionCount, as it smalls. (not very often the case though) Thus the calculated RTP changes for the real scenario and the likelihood of winning more than the 3 number increases, and the protocol may lose its funds.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider handling the small numbers or putting some boundaries for that.

c4-judge commented 1 year ago

thereksfour marked the issue as primary issue

c4-judge commented 1 year ago

thereksfour marked the issue as duplicate of #478

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Invalid