Submitted by leastwood, also found by cmichel and kirk-baird
The ConvexStakingWrapper.sol implementation makes several modifications to the original design. One of the key changes is the ability to add multiple pools into the wrapper contract, where each pool is represented by a unique _pid. By doing this, we are able to aggregate pools and their LP tokens to simplify the token distribution process.
However, the interdependence between pools introduces new problems. Because the original implementation uses the contracts reward token balance to track newly claimed tokens, it is possible for a malicious user to abuse the unguarded getReward function to maximise the profit they are able to generate. By calling getReward on multiple pools with the same reward token (i.e. cvx), users are able to siphon rewards from other pools. This inevitably leads to certain loss of rewards for users who have deposited LP tokens into these victim pools. As crv and cvx are reward tokens by default, it is very likely that someone will want to exploit this issue.
Lets consider the following scenario:
Altogether, this will lead to the loss of rewards for other stakers as they are unable to then claim their rewards.
ConvexStakingWrapper.sol#L216-L259
Manual code review.
Confirmation from Taek.
Consider redesigning this mechanism such that all pools have their getReward function called in _checkpoint. The _calcRewardIntegral function can then ensure that each pool is allocated only a fraction of the total rewards instead of the change in contract balance. Other implementations might be more ideal, so it is important that careful consideration is taken when making these changes.
leekt (Concur) confirmed
Alex the Entreprenerd (judge) commented:
