code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/AutoPxGlp.sol#L197-L296 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/AutoPxGmx.sol#L230-L313

Vulnerability details

Impact

Function compound() in the AutoPxGlp and AutoPxGmx contracts is claiming all the rewards of the pool then swaps them if required and then deposits them in the PirexGmx again to compound the reward. when swapping called, code uses the slippage tolerance values that user specified This creates an issue where a single user specify slippage for all the pool contract rewards which belongs to all of the users. attacker can use this to steal contract rewards by creating a smart contract that first manipulate the pool in Uniswap (or any pool that GMX or Pirex protocol uses in depositing) then call compound() with very high slippage allowance (set amountOutMinimum very low) and code would swap rewards token with very bad price and then attacker contract would perform the reverse transaction in Uniswap.

Proof of Concept

This is compound() code in AutoPxGmx contract:

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

As you can see contract swaps the gmxBaseReward balance of contract for gmx token by calling IV3SwapRouter and the slippage parameters for swaps amountOutMinimum and sqrtPriceLimitX96 are set by the values user provided. This would give attacker to perform on-chain sandwich attack against the pool. to perform this attack, attacker would create a smart contract would perform this steps:

get a very large number of gmxBaseReward token as flash loan.
manipulate the uniswap pool (or any pool that GMX or Pirex protocol uses when compounding or depositing) and creates a bad swap ratio by swapping very large number of gmxBaseReward tokens to gmx tokens.
call compound() function in AutoPxGmx with high slippage allowance and the code would tries to swap gmxBaseReward token for gmx token but because the liquidity pool has been manipulated AutoPxGmx would receive smaller amount of gmx token that it was supposed to.
perform the reverse of step 2 transaction, swap the gmx tokens for gmxBaseReward tokens and receive extra tokens which belonged to AutoPxGmx.

By performing this attacker was abled to steal AutoPxGmx rewards with on-chain sandwich-attack in one transaction. The situation for AutoPxGlp is similar to this. the problem is that contract allows a user to chose slippage allowance for all the rewards of contract which belongs to all the users.

Tools Used

VIM