Zellic - Audit Reports

Description

This project is a fork of GMX V1, forked at commit 58c4ac801a95236db24c0857f820bdf4935d5814. The fork changes a few things.

In BasePositionManager and PositionUtils, a few calls into the Timelock contract were commented out, in order to remove the timelock governance functionality from the system:

-address timelock = IVault(_vault).gov();
+// address timelock = IVault(_vault).gov();

 // should be called strictly before position is updated in Vault
 IShortsTracker(shortsTracker).updateGlobalShortData(_account, _collateralToken, _indexToken, _isLong, _sizeDelta, markPrice, false);

-ITimelock(timelock).enableLeverage(_vault);
+// ITimelock(timelock).enableLeverage(_vault);
 uint256 amountOut = IRouter(router).pluginDecreasePosition(_account, _collateralToken, _indexToken, _collateralDelta, _sizeDelta, _isLong, _receiver);
-ITimelock(timelock).disableLeverage(_vault);
+// ITimelock(timelock).disableLeverage(_vault);

In Vault, for long positions, the check that the collateral token and index token are the same asset was removed:

 function _validateTokens(address _collateralToken, address _indexToken, bool _isLong) private view {
     if (_isLong) {
-        _validate(_collateralToken == _indexToken, 42);
+        // _validate(_collateralToken == _indexToken, 42);
         _validate(whitelistedTokens[_collateralToken], 43);
         _validate(!stableTokens[_collateralToken], 44);
+        _validate(!stableTokens[_indexToken], 47);
         return;
     }

    _validate(whitelistedTokens[_collateralToken], 45);
    _validate(stableTokens[_collateralToken], 46);
    _validate(!stableTokens[_indexToken], 47);
    _validate(shortableTokens[_indexToken], 48);
}

In the GLP token contract, some strings were changed to rename the token to "Cultured LP".

Code was removed from the RewardRouter contract, including calls to stakedGlpTracker:

function mintAndStakeGlpETH(
    uint256 _minUsdg,
    uint256 _minGlp
) external payable nonReentrant returns (uint256) {
    // [...]
-   IRewardTracker(stakedGlpTracker).stakeForAccount(account, account, feeGlpTracker, glpAmount);

function unstakeAndRedeemGlp(
    address _tokenOut,
    uint256 _glpAmount,
    uint256 _minOut,
    address _receiver
) external nonReentrant returns (uint256) { 
    // [...]
-   IRewardTracker(stakedGlpTracker).unstakeForAccount(account, feeGlpTracker, _glpAmount, account);

     function claim() external nonReentrant {
         address account = msg.sender;
 
-        IRewardTracker(feeGmxTracker).claimForAccount(account, account);
         IRewardTracker(feeGlpTracker).claimForAccount(account, account);
-
-        IRewardTracker(stakedGmxTracker).claimForAccount(account, account);
-        IRewardTracker(stakedGlpTracker).claimForAccount(account, account);
-    }

function claimFees() external nonReentrant {
    address account = msg.sender;

-   IRewardTracker(feeGmxTracker).claimForAccount(account, account);
    IRewardTracker(feeGlpTracker).claimForAccount(account, account);
}

Invariants

Here is a list of the invariants that were altered by the in-scope changes.

The collateral token can now be a different token than the index token, for long positions. For an analysis of this, see Finding ref↗.

The index token now no longer needs to be a whitelisted token. As long as the address (or dummy address) is not in the stableTokens list, it is a valid index and can be accepted as an index by _validateTokens.

Test coverage

No tests were added.

Attack surface

The attack surface we considered for this audit is identical to the attack surface of GMX V1, where the trader is untrusted and may open and close exploitative positions and where the governance and price feeds are assumed to be trusted.