Config Syncer

In the constructor for the RiskOracle contract on Arbitrum Sepolia https://sepolia.arbiscan.io/address/0x526d6789fCb503F2F898f45912A7a24fe9dd48e4#code, a list of initialUpdateTypes may be passed where there are duplicate updateType strings. In this case the updateType string will be pushed to the allUpdateTypes list multiple times.

This can lead to unexpected behavior for any systems relying on the allUpdateTypes list as it will have duplicate entries.

Consider validating that no duplicate entries have been made for the updateTypes in the RiskOracle constructor.

When creating the newUpdate object to be stored the previousValue is declared as the direct previous update no matter what market or updateType the previous update acted upon.

This is incorrect or at least misleading as the previousValue serves little to no value if it merely references the previous update of any market/parameter. Instead the previousValue should pertain to the previous value of the market and updateType combination.

Query the last update for the particular market and parameter type via the latestUpdateIdByMarketAndType mapping and store this value as the previousValue.

The getLatestUpdateByType function loops backwards through the updateHistory to find the most recent matching entry for the updateType.

This can lead to an out of gas DoS for integrating contracts if a particular updateType has been performed before many other subsequent updates.

Be aware of this risk and avoid using the getLatestUpdateByType function in a Smart Contract. Otherwise consider refactoring the RiskOracle such that it stores the latestUpdateByType in a mapping so it can be easily queried.

Throughout the GMX contracts a common pattern for configuration keys is to include the market which is being targeted in the additional key data.

In the RiskOracle and ConfigSyncer it is assumed that the market which an update is supplied for is the one which it’s provided additionalData includes, however there is no validation to enforce this.

Be aware of this risk and put in place validations off-chain such that trusted parties will not commit updates for markets which are not included in the additionalData provided.

Otherwise consider implementing validations at the contract level which do not allow market configurations which do not agree with the market address provided in the additionalData. Though this is likely not realistic to validate on-chain due to the many existing arbitrary key datas + new keys which will be introduced in the future.

The following keys are not validated in the _validateRange function and belong to uint values which can potentially DoS or otherwise cause loss or harm to GMX V2 users.

• MAX_SWAP_PATH_LENGTH
• MIN_POSITION_SIZE_USD
• MAX_POSITION_IMPACT_FACTOR_FOR_LIQUIDATIONS
• MAX_ORACLE_PRICE_AGE
• MAX_ORACLE_TIMESTAMP_RANGE
• ORACLE_TIMESTAMP_ADJUSTMENT
• MAX_ORACLE_REF_PRICE_DEVIATION_FACTOR
• REQUEST_EXPIRATION_TIME
• MIN_COLLATERAL_FACTOR_FOR_OPEN_INTEREST_MULTIPLIER
• POSITION_IMPACT_FACTOR
• MAX_POSITION_IMPACT_FACTOR
• SWAP_IMPACT_FACTOR
• MAX_AUTO_CANCEL_ORDERS
• RESERVE_FACTOR
• OPEN_INTEREST_RESERVE_FACTOR
• MIN_FUNDING_FACTOR_PER_SECOND
• THRESHOLD_FOR_STABLE_FUNDING
• THRESHOLD_FOR_DECREASE_FUNDING
• PRICE_FEED_HEARTBEAT_DURATION
• All gas related keys

Carefully consider the risk of each of these keys being configured by the RiskOracle without any range validation. Where appropriate add the corresponding validations that the configured values are within an expected range.

In the _initAllowedBaseKeys several keys have been excluded which are present in the MockRiskOracle file.

Missing Keys from allowedBaseKeys:

• SWAP_FEE_FACTOR
• ATOMIC_SWAP_FEE_FACTOR
• TOKEN_TRANSFER_GAS_LIMIT
• MIN_COLLATERAL_FACTOR
• MIN_COLLATERAL_FACTOR_FOR_OPEN_INTEREST_MULTIPLIER
• MAX_PNL_FACTOR
• MIN_PNL_FACTOR_AFTER_ADL
• FUNDING_FACTOR
• FUNDING_EXPONENT_FACTOR
• THRESHOLD_FOR_STABLE_FUNDING
• THRESHOLD_FOR_DECREASE_FUNDING
• POSITION_FEE_FACTOR
• MAX_POSITION_IMPACT_FACTOR_FOR_LIQUIDATIONS
• MAX_POSITION_IMPACT_FACTOR

These keys may be desired to be configured yet have not been allowed in the _initAllowedBaseKeys function.

Also notably, The MAX_PNL_FACTOR key is present in the _validateMarketInData function, but not in the _initAllowedBaseKeys function.

Consider adding some or all of these keys to the _initAllowedBaseKeys function, and be aware of the risk if they are not currently validated in the Config._validateRange function.

A few keys have been marked as allowed which are not included in the MockRiskOracle contract. This may be entirely expected, this finding only serves to point out this detail out of an abundance of caution that some keys may be unexpectedly allowed.

• OPTIMAL_USAGE_FACTOR
• BASE_BORROWING_FACTOR
• ABOVE_OPTIMAL_USAGE_BORROWING_FACTOR

Consider if these keys are expected to be allowed in the ConfigSyncer.