`MultiRewardEscrow.lock()` can be DOS when `token` is `USDC`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardEscrow.sol#L107-L112

Vulnerability details

lock() is used to lock funds for escrow, for instance in MultiRewardStaking when there is an escrow percentage of a reward. If there is a fee, the code tries to transfer it to feeRecipient

107:     if (feePerc > 0) {
108:       uint256 fee = Math.mulDiv(amount, feePerc, 1e18);
109: 
110:       amount -= fee;
111:       token.safeTransfer(feeRecipient, fee);
112:     }

The issue is in the case of token == USDC, if feeRecipient ends up to be blacklisted by USDC, the call would systematically revert.

lock() reverting would mean claimRewards() reverting too in this case, leading to users unable to claim their USDC reward.

Impact

Medium

Tools Used

Manual review

Recommended Mitigation Steps

Consider using a try/catch for the transfer of fees, so that in the case of USDC blacklisting feeRecipient, users can still lock .

[c4-sponsor]

RedVeil marked the issue as sponsor acknowledged

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-b