Lines of code
<https://github.com/code-423n4/2022-11-redactedcartel/blob/684627b7889e34ba7799e50074d138361f0f532b/src/PirexGmx.sol#L824&gt;

Vulnerability details

Unbounded loop can block claim

Impact

There are no bounds on the number of rewardTokens in the loop, this can run out of gas due to cost of the operations.

##Proof Of Concept

  function claim(ERC20 producerToken, address user) external {
        
    {...}
           uint256 rLen = rewardTokens.length;
            // Transfer the proportionate reward token amounts to the recipient
            for (uint256 i; i < rLen; ++i) {
                ERC20 rewardToken = rewardTokens[i];
                address rewardRecipient = p.rewardRecipients[user][rewardToken];
                address recipient = rewardRecipient != address(0)
                    ? rewardRecipient
                    : user;
                uint256 rewardState = p.rewardStates[rewardToken];
                uint256 amount = (rewardState * userRewards) / globalRewards;

                if (amount != 0) {
                    // Update reward state (i.e. amount) to reflect reward tokens transferred out
                    p.rewardStates[rewardToken] = rewardState - amount;

                    producer.claimUserReward(//@audit expensive operations
                        address(rewardToken),
                        amount,
                        recipient
                    );
                }
            }
        }

<https://github.com/code-423n4/2022-11-redactedcartel/blob/684627b7889e34ba7799e50074d138361f0f532b/src/PirexGmx.sol#L824&gt;

    function claimUserReward(
        address token,
        uint256 amount,
        address receiver
    ) external onlyPirexRewards {
        if (token == address(0)) revert ZeroAddress();
        if (amount == 0) revert ZeroAmount();
        if (receiver == address(0)) revert ZeroAddress();

        (uint256 postFeeAmount, uint256 feeAmount) = _computeAssetAmounts(
            Fees.Reward,
            amount
        );

        if (token == address(pxGmx)) {
            // Mint pxGMX for the user - the analog for esGMX rewards
            pxGmx.mint(receiver, postFeeAmount);

            if (feeAmount != 0) pxGmx.mint(address(pirexFees), feeAmount);
        } else if (token == address(gmxBaseReward)) {
            gmxBaseReward.safeTransfer(receiver, postFeeAmount);

            if (feeAmount != 0)
                gmxBaseReward.safeTransfer(address(pirexFees), feeAmount);
        }

        emit ClaimUserReward(receiver, token, amount, postFeeAmount, feeAmount);
    }

Recommended Mitigation Steps

Have an upper bound on the number of orders to avoid this issue

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

All reactions