Wenwin contest - ast3ros's results

Lines of code

https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/staking/Staking.sol#L50-L52

Vulnerability details

Impact

When revenue is generated, 20% of it is deposited into staking contract by calling function Lottery.claimRewards: https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/Lottery.sol#L151-L157

if the staking contract has no tokens staked (_totalSupply = 0), the 20% revenue is not accounted for. The DAI amount will be stuck there permanently and nobody can withdraw it. https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/staking/Staking.sol#L50-L52

Proof of Concept

Scenario: after two years, all the locked tokens are released.

• All users withdraw their LOT tokens from the staking contract to trade. This means the totalSupply of staked tokens will be 0 (all are burned). https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/staking/Staking.sol#L79-L89
• An account claims the reward by calling Lottery.claimRewards with LotteryRewardType set to STAKING. https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/Lottery.sol#L151-L157
• 20% of ticket revenues is transferred to the staking contract without accounting in staking contract. The revenue will remain trapped there indefinitely. https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/staking/Staking.sol#L50-L52

Tools Used

Manual

Recommended Mitigation Steps

Implement logic to handle the case when the totalSupply of staking is 0. In that case, the revenue can be added to the overall pot instead of being sent to the staking contract.

[L-1] The amount of pre-staked tokens should be specified in the constructor of the Lottery Token contract and locked automatically

In white paper of Wenwin, 20% (200,000,000 tokens) is allocated to the team, which is pre-staked and locked for 1 year before the Initial Sale begins. https://docs.wenwin.com/wenwin-lottery/protocol-architecture/token

Instead of sending the 200 million LOT tokens to the admin account, they should be created and deposited directly into the locked staking contract. This will enhance the trust of users and investors. https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/LotteryToken.sol#L17-L20

Recommended Mitigation Steps

Mint directly the pre-staked amount to staking contract and send staked tokens into the StakedTokenLock contract.

[NC-1] Check address (0) of important input

Instance:

https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/Lottery.sol#L113-L114

[NC-2] Remove key word this in function call

In Solidity, you can call a function in the same contract by its name without using this.function. This is the recommended way to write code as it is more concise and clear.

https://github.com/code-423n4/2023-03-wenwin/blob/a260ecba74720c631ab06ecdd67167317b976b60/src/Lottery.sol#L261