Lines of code

Vulnerability details

Impact

If an ERC-777 token is used as reward token for any Staking contract in the system, that reward token can be completely drained from the Staking contract.

Proof of Concept

Re-entrancy can be done in the MultiRewardStaking.claimRewards function because of the following code:

      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);
      }

      accruedRewards[user][_rewardTokens[i]] = 0;

In the above code, the accounting for the user’s accrued rewards for a given token is done after the transfer. This does not adhere to the checks-effects-interactions principle that prevents re-entrancies. Also, the function does not use the nonReentrant modifier.

Because of these reasons, the attacker can then call claimRewards for an ERC-777 reward token and can keep calling/re-entering claimRewards until all of the reward tokens in the Staking contract are sent to him. Like all re-entrancies, it’s done within one transaction.

Tools Used

VSCode

Recommended Mitigation Steps

Add nonReentrant modifier to the function or update the accounting records before transferring out the tokens.

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

All reactions