Lucene search

K
code423n4Code4renaCODE423N4:2022-11-REDACTEDCARTEL-FINDINGS-ISSUES-349
HistoryNov 28, 2022 - 12:00 a.m.

Division by zero could cause DOS in function harvest() and claim() in PirexRewards contract

2022-11-2800:00:00
Code4rena
github.com
6
pirexrewards
harvest()
claim()
division by zero
vulnerability
pirexgmx contract

Lines of code

Vulnerability details

Impact

When functions harvest() or claim() of PirexRewards are called, they will claim rewards by calling PirexGmx.claimRewards() function. If there is any case that esGmx reward is existed but not base rewards or vice versa, the value returned from _calculateRewards() is 0 and it leads to division by zero and revert.

For example,

if (baseRewards != 0) {
    // This may not be necessary and is more of a hedge against a discrepancy between
    // the actual rewards and the calculated amounts. Needs further consideration
    rewardAmounts[0] =
        (gmxBaseRewards * baseRewards) /
        (gmxBaseRewards + glpBaseRewards); // @audit division by zero
    rewardAmounts[1] = baseRewards - rewardAmounts[0];
}

Since baseRewards is calculated by using pre and post balance, attacker can simply trigger it by transferring gmxBaseReward to the contract, passing the check if (baseRewards != 0).

Proof of Concept

Function harvest() calls claimRewards().
<https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexRewards.sol#L338-L348&gt;

function harvest()
    public
    returns (
        ERC20[] memory _producerTokens,
        ERC20[] memory rewardTokens,
        uint256[] memory rewardAmounts
    )
{
    (_producerTokens, rewardTokens, rewardAmounts) = producer
        .claimRewards();
    uint256 pLen = _producerTokens.length;

In function claimRewards(), rewards amount is calculated by substracting post balance with pre balance
<https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexGmx.sol#L785-L790&gt;

uint256 baseRewards = gmxBaseReward.balanceOf(address(this)) -
    baseRewardBeforeClaim;
uint256 esGmxRewards = stakedGmx.depositBalances(
    address(this),
    address(esGmx)
) - esGmxBeforeClaim;

Function _calculateRewards() can return zero when there is lacking rewards in distributor.
<https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexGmx.sol#L250-L254&gt;

uint256 distributorBalance = (isBaseReward ? gmxBaseReward : esGmx)
    .balanceOf(distributor);
uint256 blockReward = pendingRewards &gt; distributorBalance
    ? distributorBalance
    : pendingRewards;

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding zero check before doing the division.


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

All reactions