search

code-423n4 / 2022-02-concur-findings

2 stars 0 forks source link

Rewards get diluted because `totalAllocPoint` can only increase. #221

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol

Vulnerability details

Impact

There is no functionality for removing pools/setting pool's allocPoints. Therefore totalAllocPoint only increases and rewards for pool decreases.

Proof of Concept

Scenario:

  1. Owner adds new pool (first pool) for staking with points = 900 (totalAllocPoint=900).
  2. 1 week passes
  3. First pool staking period ends (or for other reasons that pool is not meaningfully anymore).
  4. Owner adds new pool (second pool) for staking with points = 100 (totalAllocPoint=1000)
  5. 1 block later Alice stake 10 tokens there (at the same time).
  6. 1 week passes
  7. After some time Alice claims rewards. But she is eligible only for 10% of the rewards. 90% goes to unused pool.

Tools Used

Manual review

Recommended Mitigation Steps

Add functionality for removing pool or functionality for setting pool's totalAllocPoint param.

GalloDaSballo commented 2 years ago

While the problem can seem trivial, the warden has proven that the contract can over time end up leaking excess value as any additional pool will dilute the totalAllocPoint and old pools cannot be retired.

The sponsor also confirms.

I believe the finding to be valid, but because the leak is contingent on settings, I believe Medium Severity to be more appropriate