code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L243 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L324 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L324

Vulnerability details

Impact

The fundReward() and addRewardToken() functions of MultiRewardStaking can be used to fund reward tokens. The end time for reward accrual is calculated by amount / uint256(rewardsPerSecond). The end timestamp is rounded down, and in some cases, after the final accrual, the contract may have a significant amount of remaining funds that are stuck and not rewarded to anyone.

Proof of Concept

Consider the following scenario:

Stage 1. An admin adds a new reward token with rewardsPerSecond = 100_000 dollars and the fund amount of 990_000 dollars via the addReward() function: https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L243

Stage 2. The end timestamp is calculated by 999_999_999 / 100_000 = 9999 seconds: https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L359

Stage 3. 9999 seconds passed, reward accrual have finished. The contract still has 999_999_999 % 100_000 = 99_999 dollars stuck.

Tools Used

Manual analysis

Recommended Mitigation Steps

It is recommended to send the remaining funds back to an administrator after the division in MultiRewardStaking fundReward() and addRewardToken().

c4-sponsor commented 1 year ago

RedVeil marked the issue as sponsor confirmed

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)