sherlock-audit / 2023-01-sentiment-judging

2 stars 0 forks source link

obront - GMX RewardRouter's compound() function does not return WETH #8

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

obront

medium

GMX RewardRouter's compound() function does not return WETH

Summary

When compound() is called, the RewardRouterController accounts for WETH as an incoming token. However, when GMX rewards are compounded, all value stays in the protocol and no tokens are withdrawn.

Vulnerability Detail

When a Sentiment account calls compound() on the GMX Reward Router, the Controller accounts for WETH being returned to the account:

function canCallCompound()
    internal
    view
    returns (bool, address[] memory, address[] memory)
{
    return (true, WETH, new address[](0));
}

When compound() is called, the following two functions in GMX's Reward Router are called:

    function _compoundGmx(address _account) private {
        uint256 esGmxAmount = IRewardTracker(stakedGmxTracker).claimForAccount(_account, _account);
        if (esGmxAmount > 0) {
            _stakeGmx(_account, _account, esGmx, esGmxAmount);
        }

        uint256 bnGmxAmount = IRewardTracker(bonusGmxTracker).claimForAccount(_account, _account);
        if (bnGmxAmount > 0) {
            IRewardTracker(feeGmxTracker).stakeForAccount(_account, _account, bnGmx, bnGmxAmount);
        }
    }

    function _compoundGlp(address _account) private {
        uint256 esGmxAmount = IRewardTracker(stakedGlpTracker).claimForAccount(_account, _account);
        if (esGmxAmount > 0) {
            _stakeGmx(_account, _account, esGmx, esGmxAmount);
        }
    }

As you can see, in both of these functions, once WETH has been claimed, the full amount is immediately staked back into the protocol. To confirm this, I wrote up a Foundry test that forks Arbitrum mainnet and compounds, verifying that a user's WETH balance does not increase.

You can find the test ready to copy & paste here.

Impact

WETH will be added as an asset in the user's account, even if it shouldn't be.

This specific Impact was judged as Medium for multiple issues in the previous contest:

Code Snippet

https://github.com/sherlock-audit/2023-01-sentiment/blob/main/controller-55/src/gmx/RewardRouterController.sol#L62-L68

Tool used

Manual Review

Recommendation

Remove WETH from the tokensIn when compound() is called:

function canCallCompound()
    internal
    view
    returns (bool, address[] memory, address[] memory)
{
-    return (true, WETH, new address[](0));
+    return (true, new address[](0), new address[](0));
}

Duplicate of #10

hrishibhat commented 1 year ago

Considering this issue a duplicate of #10 as the underlying issue is the same occurring in multiple places.