code-423n4 / 2022-11-redactedcartel-findings

3 stars 2 forks source link

`AutoPxGlp.compound()` might revert forever if `gmxBaseReward` is not whitelisted. #345

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/AutoPxGlp.sol#L240-L246 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexGmx.sol#L600

Vulnerability details

Impact

AutoPxGlp.compound() might revert forever if gmxBaseReward is not whitelisted in gmxVault.

As a result, most functions like withdraw() and redeem() using the compound() function inside will revert.

Proof of Concept

AutoPxGlp.compound() deposits the earned gmxBaseReward into the PirexGmx contract and mints pxGlp.

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

And PirexGmx.depositGlp() accepts the whitelisted tokens only.

    if (token == address(0)) revert ZeroAddress();
    if (!gmxVault.whitelistedTokens(token)) revert InvalidToken(token);

Also, while checking the gmxVault contract, there is no guarantee gmxBaseReward is whitelisted.

So if gmxBaseReward is not a whitelisted token in the gmxVault, compound() will revert.

Tools Used

Manual Review

Recommended Mitigation Steps

We should check if the gmxBaseReward is whitelisted or not in compound().

And should consider swapping the gmxBaseReward to the whitelisted token using a swap router.

Picodes commented 1 year ago

Considering that gmxBaseReward is GMX protocol base reward, the chances it is not whitelisted are very low

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-11-redactedcartel-findings/issues/347