Small Fees May Lead to the User Not Being Able to Claim Their Rewards for a Certain Amount of Time

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/main/src/utils/MultiRewardStaking.sol#L243-L288 https://github.com/code-423n4/2023-01-popcorn/blob/main/src/utils/MultiRewardStaking.sol#L191-L202

Vulnerability details

Impact

Owners have the freedom to choose any token as the reward token, including arbitrary tokens. Some ERC20 tokens, such as USDC, only have 6 decimal places. When tokens with less than 18 decimal places, such as USDC, are used as the reward token, there is a likelihood that the user will not be able to claim rewards if the fees are too small.

User may not be able to claim rewards for a period of time depending on the rewardsPerSecond and the escrowPercentage.

Proof of Concept

Suppose the owner adds a reward token with only 6 decimal places and selects 1e11 as the escrowPercentage, potentially due to a mistake or for any other reason. Users who are accruing tokens are unable to call the claimRewards() function if their accrued rewards are not more than 2e6.

function addRewardToken(
    IERC20 rewardToken,
    uint160 rewardsPerSecond,
    uint256 amount,
    bool useEscrow,
    uint192 escrowPercentage,
    uint32 escrowDuration,
    uint32 offset
  ) external onlyOwner {

    ...

    if (useEscrow) {
      escrowInfos[rewardToken] = EscrowInfo({
        escrowPercentage: escrowPercentage,
        escrowDuration: escrowDuration,
        offset: offset
      });
      rewardToken.safeApprove(address(escrow), type(uint256).max);
    }

    ...
  }

For example, when the rewardAmount is 0.9e6, the user calls the claimRewards() function. The _lockToken() function is executed, but the escrowed amount will be 0 as 0.9e6 * 1e11 / 1e18 rounds down to 0 after calculation.

File: src/utils/MultiRewardStaking.sol

uint256 escrowed = rewardAmount.mulDiv(uint256(escrowInfo.escrowPercentage), 1e18, Math.Rounding.Down);

This results in the user being unable to call the claimRewards() function, as it will revert when the MultiRewardEscrow.sol calls the lock() function due to if (amount == 0) revert ZeroAmount() condition.

This issue becomes even more pronounced when the fees are small and the accrued rewards rate is slow.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding a minimum fee to the function addRewardToken() for escrowPercentage().

[c4-judge]

dmvt marked the issue as duplicate of #251

[c4-sponsor]

RedVeil marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as not a duplicate

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-b