Lines of code
<https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/AutoPxGlp.sol#L232-L250&gt;

Vulnerability details

Impact

Compounding will attempt to swap/deposit all assets instead of just the rewards, which reverts because of integer overflow, which causes withdrawals to revert.

Proof of Concept

In AutoPxGmx.compound():

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

we see that if gmxBaseReward is the same token as asset (e.g. WETH, WAVAX) then the entire balance of asset will be swapped. Then the last line above if ((totalAssets() - assetsBeforeClaim) != 0) { will revert as now totalAssets() will be 0 (unless this is compounded before a first deposit). compound() is called in withdraw() and redeem() thus causing these to revert. However, a first deposit is possible since compound() doesn’t revert in beforeDeposit() at the first deposit since assets are 0.

We have the same problem in AutoPxGlp.compound():

uint256 preClaimTotalAssets = asset.balanceOf(address(this));
uint256 preClaimPxGmxAmount = pxGmx.balanceOf(address(this));

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

// Track the amount of rewards received
gmxBaseRewardAmountIn = gmxBaseReward.balanceOf(address(this));

if (gmxBaseRewardAmountIn != 0) {
    // Deposit received rewards for pxGLP
    (, pxGlpAmountOut, ) = PirexGmx(platform).depositGlp(
        address(gmxBaseReward),
        gmxBaseRewardAmountIn,
        minUsdg,
        minGlp,
        address(this)
    );
}

// Distribute fees if the amount of vault assets increased
uint256 newAssets = totalAssets() - preClaimTotalAssets;

Tools Used

Code inspection

Recommended Mitigation Steps

Check balance before and take the difference of this and the new balance.

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

All reactions