code-423n4 / 2023-01-popcorn-findings

0 stars 0 forks source link

Small rewards from MultiRewardStaking spread out over multiple safes in MultiRewardEscrow can lead to a loss of user interest in claiming them #819

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L170 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L179 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L197 https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L201

Vulnerability details

Impact

The MultiRewardStaking claimRewards() function can be publicly called by any user for any account. It in turn calls _lockToken(), which puts the accumulated rewards for the account into a separate escrow safe. However, if there are a lot of shares minted in MultiRewardStaking and the user holds a small portion of shares, they will receive very small reward over time. If the user themselves or another actor repeatedly calls the claimRewards() function for that user, escrow safes will be created with too small a reward to cover the total gas expenses for the user to extract them.

Proof of Concept

The claimRewards() function can be publicly called on any block for any account: https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L170

Escrowed token rewards are locked via _lockToken(): https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L179

Even if a user had a small reward in the first place, the locked value may get even smaller by getting a percentage of initial value: https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L197

Final value is locked in a separate safe: https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L201

Tools Used

Manual analysis

Recommended Mitigation Steps

One way to resolve the problem is to skip tokens with small lock values accrued in claimRewards() until the value gets over some threshold.

c4-sponsor commented 1 year ago

RedVeil marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

dmvt changed the severity to G (Gas Optimization)

c4-judge commented 1 year ago

dmvt marked the issue as grade-b

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)