Redacted Cartel contest - wagmi's results

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/AutoPxGmx.sol#L275

Vulnerability details

Impact

Function compound() is called in every deposit/withdraw in AutoPxGmx contract. It claim rewards in form of gmxBaseReward and swap them to gmx token. They used amountOutMinimum from input params

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

However amountOutMinimum always has the value 1 when called by withdraw() or deposit() function.

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

An attacker can front-run swap in Router before compound() to make a profit.

Proof of Concept

Please refer to yDai Incident to check the severity of a harvest function without slippage control.

Please refer to Mushrooms-finance-theft to check how likely this kind of attack might happen.

Recommended Mitigation Steps

Consider adding using some fixed lowerbound rate (since it's trading from pxGMX to GMX so the rate should not be volatile)