StakingOperations.claim doesn't update reward debt, allowing multiple claims

criticalRecon Audits

Impact

solidity📋
  function claim(ISwapPair _pid) external override {
    requireValidPool(_pid);
    _claim(_pid, msg.sender);
  }

  function batchClaim(ISwapPair[] memory _pids) external override {
    uint length = _pids.length;
    for (uint n = 0; n < length; n++) {
      requireValidPool(_pids[n]);
      _claim(_pids[n], msg.sender);
    }

User rewards are computed as ((user.amount * accRewardPerShare) / REWARD_DECIMALS) - user.rewardDebt;

user.rewardDebt = (user.amount * pool.accRewardPerShare) / REWARD_DECIMALS; is set in deposit and withdraw

but it is not update on claim and batchClaim

Due to this, an exploiter can simply call claim and batchClaim repeatedly and steal all rewards

Mitigation

Change _claim to:

• Calculate user rewards and cache that value
• Set user debt to the new value, so their pendingReward are now zero
• Then transfer the tokens

Also consider implementing the following invariants:

• pendingReward are set to zero after a call to claim and batchClaim