Lines of code
<https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L362&gt;
<https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L313&gt;

Vulnerability details

Impact

As shown below, calling the AutoPxGmx.withdraw and AutoPxGmx.redeem functions would execute compound(poolFee, 1, 0, true), which uses the hardcoded 1 as the amountOutMinimum input of the AutoPxGmx.compound function to further call the SWAP_ROUTER.exactInputSingle function. However, calling the SWAP_ROUTER.exactInputSingle function with amountOutMinimum being hardcoded to 1 does not provide an effective slippage control, which is unlike the case where the AutoPxGmx.compound function could be called directly with a specified amountOutMinimum that provides an effective slippage control. When the Uniswap V3 pool becomes relatively imbalanced comparing to when the pool is relatively balanced, executing compound(poolFee, 1, 0, true), which does not provide an effective slippage control, can result in a much smaller minted pxGMX reward amount due to a much smaller GMX amount outputted after swapping the same input WETH reward amount; as a result, calling functions like AutoPxGmx.withdraw and AutoPxGmx.redeem in this situation can result in a much smaller withdrawn pxGMX amount.

<https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L315-L337&gt;

    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) public override returns (uint256 shares) {
        // Compound rewards and ensure they are properly accounted for prior to withdrawal calculation
        compound(poolFee, 1, 0, true);

        shares = previewWithdraw(assets); // No need to check for rounding error, previewWithdraw rounds up.

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max)
                allowance[owner][msg.sender] = allowed - shares;
        }

        _burn(owner, shares);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        asset.safeTransfer(receiver, assets);
    }

<https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L362&gt;

    function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public override returns (uint256 assets) {
        // Compound rewards and ensure they are properly accounted for prior to redemption calculation
        compound(poolFee, 1, 0, true);

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max)
                allowance[owner][msg.sender] = allowed - shares;
        }

        // Check for rounding error since we round down in previewRedeem.
        require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS");

        _burn(owner, shares);

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        asset.safeTransfer(receiver, assets);
    }

<https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L313&gt;

    function compound(
        uint24 fee,
        uint256 amountOutMinimum,
        uint160 sqrtPriceLimitX96,
        bool optOutIncentive
    )
        public
        returns (
            uint256 gmxBaseRewardAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        )
    {
        if (fee == 0) revert InvalidParam();
        if (amountOutMinimum == 0) revert InvalidParam();

        uint256 assetsBeforeClaim = asset.balanceOf(address(this));

        PirexRewards(rewardsModule).claim(asset, address(this));

        // Swap entire reward balance for GMX
        gmxBaseRewardAmountIn = gmxBaseReward.balanceOf(address(this));

        if (gmxBaseRewardAmountIn != 0) {
            gmxAmountOut = SWAP_ROUTER.exactInputSingle(
                IV3SwapRouter.ExactInputSingleParams({
                    tokenIn: address(gmxBaseReward),
                    tokenOut: address(gmx),
                    fee: fee,
                    recipient: address(this),
                    amountIn: gmxBaseRewardAmountIn,
                    amountOutMinimum: amountOutMinimum,
                    sqrtPriceLimitX96: sqrtPriceLimitX96
                })
            );

            // Deposit entire GMX balance for pxGMX, increasing the asset/share amount
            (, pxGmxMintAmount, ) = PirexGmx(platform).depositGmx(
                gmx.balanceOf(address(this)),
                address(this)
            );
        }

        // Only distribute fees if the amount of vault assets increased
        if ((totalAssets() - assetsBeforeClaim) != 0) {
            totalFee =
                ((asset.balanceOf(address(this)) - assetsBeforeClaim) *
                    platformFee) /
                FEE_DENOMINATOR;
            incentive = optOutIncentive
                ? 0
                : (totalFee * compoundIncentive) / FEE_DENOMINATOR;

            if (incentive != 0) asset.safeTransfer(msg.sender, incentive);

            asset.safeTransfer(owner, totalFee - incentive);
        }

        ...
    }

Proof of Concept

Please add the following code in test\AutoPxGmx.t.sol.

1. Import IWETH and IV3SwapRouter as follows.

import {IWETH} from "src/interfaces/IWETH.sol";
import {IV3SwapRouter} from "src/interfaces/IV3SwapRouter.sol";

2. Add the following uint32, uint96, and uint256 variables and testCompoundWhenPoolIsRelativelyBalanced and testCompoundWhenPoolIsRelativelyImbalanced tests. Both tests will pass to demonstrate the described scenario. Please see the comments in these tests for more details.

   uint96 gmxAmount__ = 1e18;
   uint32 secondsElapsed__ = 3600;

   uint256 wethAmtIn__ = 598927240182;
   uint256 gmxAmtOutRelativelyBalancedPool = 17947773728826;
   uint256 pxGmxMintAmtRelativelyBalancedPool = 17947773728826;

   function testCompoundWhenPoolIsRelativelyBalanced() external {
   address[] memory receivers = new address;
   receivers[0] = address(this);
   provisionRewardState(gmxAmount_, receivers, secondsElapsed__);

      // functions like AutoPxGmx.withdraw executes compound(poolFee, 1, 0, true), which is same as the following compound function call
      vm.prank(testAccounts[0]);
      (
          uint256 wethAmountIn,
          uint256 gmxAmountOut,
          uint256 pxGmxMintAmount,
          uint256 totalFee,
          uint256 incentive
      ) = autoPxGmx.compound(autoPxGmx.poolFee(), 1, 0, true);
   
      // When the Uniswap V3 pool is relatively balanced, the 598927240182 WETH reward amount is swapped to 17947773728826 GMX via Uniswap V3,
      //   and 17947773728826 pxGMX reward amount is minted via PirexGmx.
      assertEq(wethAmountIn, wethAmtIn__);
      assertEq(gmxAmountOut, gmxAmtOutRelativelyBalancedPool);
      assertEq(pxGmxMintAmount, pxGmxMintAmtRelativelyBalancedPool);
   

   }

   function testCompoundWhenPoolIsRelativelyImbalanced() external {
   // the Uniswap V3 pool becomes relatively imbalanced after 1_000_000 ether WETH is swapped for GMX
   uint256 wethAmtInSwap = 1_000_000 ether;

      vm.startPrank(address(this));
   
      vm.deal(address(this), wethAmtInSwap);
      IWETH(address(weth)).deposit{value: wethAmtInSwap}();
      weth.approve(address(autoPxGmx.SWAP_ROUTER()), type(uint256).max);
   
      autoPxGmx.SWAP_ROUTER().exactInputSingle(
          IV3SwapRouter.ExactInputSingleParams({
              tokenIn: address(weth),
              tokenOut: address(gmx),
              fee: autoPxGmx.poolFee(),
              recipient: address(this),
              amountIn: wethAmtInSwap,
              amountOutMinimum: 1,
              sqrtPriceLimitX96: 0
          })
      );
   
      vm.stopPrank();
   
      // the inputs used for calling the _provisionRewardState function are same as these used in the testCompoundWhenPoolIsRelativelyBalanced test
      address[] memory receivers = new address[](1);
      receivers[0] = address(this);
      _provisionRewardState(gmxAmount__, receivers, secondsElapsed__);
   
      // functions like AutoPxGmx.withdraw executes compound(poolFee, 1, 0, true), which is same as the following compound function call
      vm.prank(testAccounts[0]);
      (
          uint256 wethAmountIn,
          uint256 gmxAmountOut,
          uint256 pxGmxMintAmount,
          uint256 totalFee,
          uint256 incentive
      ) = autoPxGmx.compound(autoPxGmx.poolFee(), 1, 0, true);
   
      // When the pool is relatively imbalanced,
      //   the 598927240182 WETH reward amount, which is the same WETH reward amount used in the testCompoundWhenPoolIsRelativelyBalanced test,
      //   is swapped to only 141 GMX that is much less than the 17947773728826 output GMX amount when the pool is relatively balanced.
      assertEq(wethAmountIn, wethAmtIn__);
      assertEq(gmxAmountOut, 141);
      assertLt(gmxAmountOut, gmxAmtOutRelativelyBalancedPool);
   
      // in this case, the minted pxGMX reward amount is also only 141 that is much less than the 17947773728826 minted pxGMX reward amount when the pool is relatively balanced
      assertEq(pxGmxMintAmount, 141);
      assertLt(pxGmxMintAmount, pxGmxMintAmtRelativelyBalancedPool);
   
      // Hence, when the pool is relatively imbalanced comparing to when the pool is relatively balanced,
      //   because executing compound(poolFee, 1, 0, true), which does not provide an effective slippage control,
      //   can result in a much smaller minted pxGMX reward amount for the same swapped input WETH reward amount,
      //   calling functions like AutoPxGmx.withdraw can result in a much smaller withdrawn pxGMX amount.
   

   }

Tools Used

VSCode

Recommended Mitigation Steps

Functions like AutoPxGmx.withdraw and AutoPxGmx.redeem can be updated to include an amountOutMinimum input, which is similar to the AutoPxGmx.compound function. When this amountOutMinimum input is specified, this specified amountOutMinimum value would be used for further calling the AutoPxGmx.compound function; otherwise, 1 can be used as the amountOutMinimum input value for further calling the AutoPxGmx.compound function.

---

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

All reactions