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

1 stars 1 forks source link

Ticket may expire sooner or later than after one year #465

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L159-L168 https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/Lottery.sol#L271-L277 https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/LotteryMath.sol#L24 https://github.com/code-423n4/2023-03-wenwin/blob/91b89482aaedf8b8feb73c771d11c257eed997e8/src/LotterySetup.sol#L41-L102

Vulnerability details

Impact

Ticket may expire sooner or later than after one year. Especially, they may expire sooner than the player expects, leading to a loss of winnings. It may similarly return the jackpot too soon or too late.

Proof of Concept

Tickets are only claimable within, and an unclaimed jackpot returned to the pot after, DRAWS_PER_YEAR draws. This is hardcoded to 52, which assumes a draw period of one week. However, drawPeriod is freely set. This means that the expiry date of 52 draws may be vastly different from one year, as suggested by the variable name as well as the docs.

Tools Used

Code inspection

Recommended Mitigation Steps

Let DRAWS_PER_YEAR be an immutable constant calculated based on drawPeriod.

thereksfour commented 1 year ago

Submit QA as High Risk

c4-judge commented 1 year ago

thereksfour marked the issue as unsatisfactory: Overinflated severity

c4-judge commented 1 year ago

thereksfour marked the issue as satisfactory

c4-judge commented 1 year ago

thereksfour marked the issue as duplicate of #408

c4-judge commented 1 year ago

thereksfour changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

thereksfour marked the issue as grade-c