Description

The VaultPriceFeed contract is used to determine the pricing of the indexToken in a specified vault. The VaultPriceFeed in GMX is supposed to track specific on-chain assets, so its behavior may differ when taken outside of that functionality. Plume intends to use arbitrary (potentially off-chain) assets as suitable index tokens.

There are several potentially dangerous defaults set on the VaultPriceFeed that may not apply to Plume's use cases.

The Vault.getMaxPrice() function that is used on getRedemptionAmount(), usdToTokenMin(), getGlobalShortDelta(), and getDelta() passes an _includeAmmPrice == true. When combined with default parameters for the VaultPriceFeed.isAmmEnabled = true and isSecondaryPriceEnabled = true, the VaultPriceFeed.getPriceV1() may default to alternative pricing mechanisms that may not apply to off-chain index tokens.

The getPriceV1() function increments the returned price as follows:

uint256 price = getPrimaryPrice(_token, _maximise);

if (_includeAmmPrice && isAmmEnabled) {
    uint256 ammPrice = getAmmPrice(_token);
    if (ammPrice > 0) {
        if (_maximise && ammPrice > price) {
            price = ammPrice;
        }
        if (!_maximise && ammPrice < price) {
            price = ammPrice;
        }
    }
}

if (isSecondaryPriceEnabled) {
    price = getSecondaryPrice(_token, price, _maximise);
}

Provided the token address matches the stored BNB, ETH, or BTC values, the getAmmPrice() will attempt to fetch the price from a necessary price from the PancakePair AMM reserve ratios. However, the default values for BNB, ETH, and USD are all address(0). This suggests a token index of address(0) may incorrectly get mapped to default values of actual on-chain assets.

Impact

Through a minor misconfiguration during deployment, it is possible that pricing mechanisms reflect on-chain assets as opposed to desired off-chain tickers. This could result in gaming certain indexToken positions.

Recommendations

We would recommend disabling certain features by default, such as isAmmEnabled and potentially isSecondaryPriceEnabled, if the contracts will always be used with custom price feeds. Furthermore, we recommend preventing privileged users from enabling priceFeeds[token] where token matches the address of any BNB, ETH, or BTC values.

Remediation

This issue has been acknowledged without a confirmed fix implemented by Plume Network. Plume Network has stated that isAmmEnabled and isSecondaryPriceEnabled will be disabled as recommended.