code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

If the protocol is not able to initially gather minInitialPot of funds, it will enter a frozen state and all gathered funds will be forever stuck in the contract.

Proof of Concept

When setting up the lottery, finalizeInitialPotRaise function in LotterySetup only initializes the lottery if minInitialPot funds have been sent to the contract.

        uint256 raised = rewardToken.balanceOf(address(this));
        if (raised < minInitialPot) {
            revert RaisedInsufficientFunds(raised);
        }

minInitialPot is set during deployment and is immutable. If for unfortunate reasons, the protocol is not able to gather enough funds, the funds that have already been sent will be forever stuck in the contract, unable to be withdrawn. There is no method in the contract to rescue these funds.

We think that this is a High severity issue, because it causes fund loss to users or protocol.

Tools Used

VS code, Manual analysis

Recommended Mitigation Steps

Few possible mitigations can be considered: a) Switch to a transferFrom-pull-mechanism, and note how much money each entity contributed. Then, if the pot is not finalized until the first draw, these entities can withdraw their funds. b) The same entity that can change the random number source, may be allowed to withdraw the funds if the pot is not finalized until the first draw. c) Or that entity might be allowed to change minInitialPot.

thereksfour commented 1 year ago

minInitialPot generally small, downgraded to Med, may consider QA

        uint256 tokenUnit = 10 ** IERC20Metadata(address(lotterySetupParams.token)).decimals();
        minInitialPot = 4 * tokenUnit;