Lines of code

Vulnerability details

Impact

The claimRewards function in the MultiRewardStaking contract is used by users to claim token rewards, but because the function does not contain a nonReentrant modifier and does not implement the CEI standard (check-effect-interact) it can be subject to reentrancy attacks.

In fact any user can call the function to claim a given amount of reward tokens and then immediately reenter the function to claim more tokens than his actual accrued rewards amount and he can even drain the whole contract token balance.

Proof of Concept

The issue occurs in the claimRewards function :

File: utils/MultiRewardStaking.sol Line 170-188

function claimRewards(address user, IERC20[] memory _rewardTokens) external accrueRewards(msg.sender, user) {
    for (uint8 i; i < _rewardTokens.length; i++) {
      uint256 rewardAmount = accruedRewards[user][_rewardTokens[i]];
    
      if (rewardAmount == 0) revert ZeroRewards(_rewardTokens[i]);
    
      EscrowInfo memory escrowInfo = escrowInfos[_rewardTokens[i]];
    
      if (escrowInfo.escrowPercentage > 0) {
        _lockToken(user, _rewardTokens[i], rewardAmount, escrowInfo);
        emit RewardsClaimed(user, _rewardTokens[i], rewardAmount, true);
      } else {
        _rewardTokens[i].transfer(user, rewardAmount);
        emit RewardsClaimed(user, _rewardTokens[i], rewardAmount, false);
      }
        
      /** @audit
        should be placed before the token transfer
      */    
      accruedRewards[user][_rewardTokens[i]] = 0;
    }
}

As you can see the function updates the user’s accrued balance value after the rewards transfer thus the user can reenter the function multiple times to claim more rewards than he is supposed to and he can by doing so drain the contract balance for a given token.

Tools Used

Manula review

Recommended Mitigation Steps

To avoid this issue add a nonReentrant modifier to the claimRewards function and follow the CEI standard :

function claimRewards(address user, IERC20[] memory _rewardTokens) external nonReentrant accrueRewards(msg.sender, user) {
    for (uint8 i; i < _rewardTokens.length; i++) {
      uint256 rewardAmount = accruedRewards[user][_rewardTokens[i]];
    
      if (rewardAmount == 0) revert ZeroRewards(_rewardTokens[i]);
    
      EscrowInfo memory escrowInfo = escrowInfos[_rewardTokens[i]];
      
      /** @audit
        update the user accrued rewards immediately
      */    
      accruedRewards[user][_rewardTokens[i]] = 0;
      
      if (escrowInfo.escrowPercentage > 0) {
        _lockToken(user, _rewardTokens[i], rewardAmount, escrowInfo);
        emit RewardsClaimed(user, _rewardTokens[i], rewardAmount, true);
      } else {
        _rewardTokens[i].transfer(user, rewardAmount);
        emit RewardsClaimed(user, _rewardTokens[i], rewardAmount, false);
      }
    }
}

The text was updated successfully, but these errors were encountered:

All reactions