DexodusV2 - Dexodus

The setPRICE_BAND_WIDTH function in Automation contracts allows arbitrary updates to the PRICE_BAND_WIDTH value.

Changing this value dynamically can lead to inconsistent band indexing for existing positions because:

• Old positions remain mapped to bands calculated with the previous width, while new positions are mapped based on the updated width.

• Positions that should be in separate bands may get merged into the same band, resulting in missed liquidations, incorrect executions, or skipped processing during upkeep.

Example:

• Initial PRICE_BAND_WIDTH = 200, placing a position at index 2 (400).

• Updated PRICE_BAND_WIDTH = 100, causing positions at 400 to map to index 4 (400/100).

• Result: Old and new positions incorrectly share the same band, leading to unstable system behavior.

1. Remove setPRICE_BAND_WIDTH: Prevent runtime modifications to PRICE_BAND_WIDTH entirely, enforcing immutability for consistent indexing.

2. Add Migration Function (Optional): If updates are required, introduce a migration process to recalculate and remap all positions to new bands. However, this approach may be gas-intensive and should be carefully considered.

3. Off-Chain Band Management (Alternative): Consider managing band calculations off-chain and only submitting validated liquidation data on-chain, reducing complexity and gas usage.

Restricting dynamic changes or implementing migrations ensures system stability, data consistency, and reliable liquidations.

SOLVED: The setPRICE_BAND_WIDTH function was removed.

The order function does not validate the existence of a position for decrease orders (orderType != 0). This allows creating orders without a valid position, leading to:

1. Underflow Errors: Invalid size or collateral can cause reverts during modifyPosition execution.

2. DOS Vulnerability: Attackers can spam invalid orders, overloading the system and blocking valid ones.

Add a position validation check for orderType != 0 to ensure the position exists and has sufficient collateral and size. This prevents invalid orders from being processed and reduces the risk of DOS attacks and underflows. Adding spam protection or rate limits can further secure the system.

SOLVED: The Automation contracts are performing an existence check via the position key and its size.

The modifyPosition function does not validate whether the specified size parameter is greater than the actual position size, especially after calculating netPnL.

If a size larger than the current position size is used, an underflow can occur during the calculation of position.size and related operations. This could result in unexpected behavior, including contract reverts or inconsistencies in position states.

The issue is particularly concerning after the if (!liq) block, where netPnL, positivePnL, and takerFee adjustments are applied without validating that the remaining size is sufficient for the operation.

Add a validation check to ensure the requested size does not exceed the current position size, accounting for positivePnL and takerFee. For example:

require(size <= position.size - takerFee - positivePnL, "Invalid size: exceeds position size");

This check should be placed after the netPnL calculation and before any updates to position.size. This approach prevents unhandled underflows and ensures that position modifications are executed safely.

SOLVED: The check was added

The checkCallback function in AutomationLiquidationsV2, AutomationOrdersV2, and similar contracts only checks for liquidations within a fixed 3-band window (current band and ±1 adjacent bands).

If the price moves rapidly and skips more than 2 bands, positions in skipped bands are not checked for liquidation, leading to:

1. Missed Liquidations: Positions in skipped bands remain open even if they should be liquidated.

2. Unstable System State: Unliquidated positions can lead to unrealistic profit/loss calculations, affecting system stability and user balances.

3. Security Risks: Malicious actors could exploit rapid price changes to bypass liquidation checks and maintain unsafe positions.

1. Implement Dynamic Band Range Checking:
   Track the last checked price band and dynamically calculate the range of bands to check based on the current price band. Iterate over all bands between the last checked band and the current band.

2. Cache Last Checked Band:
   Maintain a state variable to cache the last checked band, enabling dynamic band traversal even after rapid price changes.

This approach prevents missed liquidations, ensures system stability, and protects against edge-case exploits caused by rapid price movements.

RISK ACCEPTED: Recommendations will be implemented in the near future. For now, missed liquidations and order executions caused by rapid price movements will be executed by the off-chain operators.

The updateUserAffiliate and setReferralsSettings functions do not validate that the affiliateReceives, referrerReceives, and hasReferrerDiscount values are within the expected basis points limit of 1e10.

This lack of validation can lead to:

1. Overflow of Basis Points - Values greater than 1e10 may be incorrectly interpreted, leading to unrealistic percentages, miscalculations, or logical errors.

2. Unintended Rewards or Discounts - Excessive rewards or discounts may be assigned, resulting in financial losses or unfair advantages for specific users.

Add validation checks to enforce the upper limit for the provided values. For example:

require(affiliateReceives[i] <= 1e10, "Invalid affiliateReceives value");
require(_hasReferrerDiscount <= 1e10, "Invalid hasReferrerDiscount value");
require(_referrerReceives <= 1e10, "Invalid referrerReceives value");

These checks should be added at the beginning of the affected functions to prevent setting invalid values. This ensures consistency with the expected basis points format and avoids errors in calculations.

SOLVED: The checks have been added.

The getVipLevelDiscount function returns a discount based on the user’s VIP level, and the hasReferrerDiscount applies an additional discount. However, there is no validation to ensure that their combined value does not exceed REFERRAL_BASIS_POINTS (1e10).

If the total discount surpasses this limit, it may lead to underflows during fee calculations in the _applyFeeDistribution function of the FuturesCore contract, potentially resulting in errors or unexpected behavior.

Add checks to ensure that the sum of discounts, including VIP level and referrer discounts, does not exceed REFERRAL_BASIS_POINTS. It is also recommended to validate input values when setting discounts to prevent invalid configurations that may cause calculation errors.

SOLVED: A check has been added to verify the finalFee.

The createReferralCustomName function allows an empty string as a referral name. This leads to the referrerNameToId mapping being set with the user’s referral ID under an empty key. Consequently, the empty string could be used as a valid name, potentially allowing conflicts, incorrect lookups, or bypassing name uniqueness checks.

Add a validation check to ensure that the name parameter is not an empty string before processing the request. It is also advisable to enforce additional constraints, such as minimum length or valid character checks, to prevent invalid entries.

SOLVED: A zero length check was added.

When initializing a market in the initializeMarket function, the sum of tradingFeeToLPs and tradingFeeToTreasury is not validated to ensure it equals 100% (in basis points). If their combined value exceeds 100%, users may be charged more fees than expected when performing actions such as modifyPosition or causing overflows.

Additionally, if the sum is less than 100%, it could result in incorrect fee distribution, potentially leading to accounting discrepancies between the protocol and users.

Add a validation check in the initializeMarket function to ensure that the sum of tradingFeeToLPs and tradingFeeToTreasury equals BASIS_POINTS (10,000). For example:

require(tradingFeeToLPs + tradingFeeToTreasury == BASIS_POINTS, "Fees must add up to 100%");

This enforces consistency in fee distribution and prevents overcharging or undercharging users during position modifications and other actions.

SOLVED: The initializeMarket function is now verifying that the fees add up 100%.

The updateUserVipLevel, updateUserAffiliate, and setVipLevelsDiscount functions fail to validate that the input arrays _users and vipLevel, _users and affiliateReceives, and vipLevel and discount have matching lengths. This can lead to the following issues:

1. Incomplete Updates - If one array is shorter than the other, updates or assignments may be skipped, leaving some values uninitialized or unchanged.

2. Reverts - If the first array (used for iteration) is longer, attempts to access out-of-bound indices in the second array can trigger runtime reverts, resulting in a denial of service.

Such behavior could lead to incomplete or inconsistent data states, which may be exploited to create discrepancies in user permissions or rewards.

Add a validation step at the beginning of each function to ensure that all input arrays have the same length. For example:

require(_users.length == vipLevel.length, "Input arrays length mismatch");

Similar checks should be implemented for all affected functions. This ensures data consistency and prevents runtime errors during execution.

SOLVED: Array length validation is not performed

The public modifyPosition function does not validate its input parameters, such as ensuring slippage is less than 10000 or checking other parameter ranges for validity.

Without proper validation in the public function, invalid parameters may pass through and require Chainlink automation to handle them. This results in unnecessary automation triggers, only for the transaction to eventually revert due to invalid inputs, wasting gas and resources.

Additionally, the lack of validation in the public function means that automations rely on redundant checks inside internal functions, which could be skipped if parameters were pre-validated.

Validate all parameters in the public modifyPosition function to ensure they are within acceptable ranges before any further processing.

By enforcing these checks early, invalid transactions are filtered out without triggering automation, improving efficiency and reducing gas costs. Once validations are implemented in the public function, redundant checks inside automation handler functions can be safely removed.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

In the modifyPosition function, within the !isIncrease && !liq block, the calculation of fees distributed to the Liquidity Provider (LP) and Treasury includes the usdcRewardAmount rewarded to the referrer.

This behavior differs from the isIncrease && !liq case, where only the takerFee is used to calculate LP and Treasury fees, excluding the usdcRewardAmount.

As a result, LP and Treasury fees are effectively applied to the referrer rewards, leading to inconsistent fee distribution logic and potentially higher fees than intended for users closing or reducing positions.

Align the fee distribution logic in the !isIncrease && !liq case with the isIncrease && !liq case. Only use the takerFee (excluding usdcRewardAmount) when calculating fees for the LP and Treasury.

For example, update the logic to:

uint256 lpFee = takerFee * marketsState[key].tradingFeeToLPs / BASIS_POINTS;
uint256 treasuryFee = takerFee * marketsState[key].tradingFeeToTreasury / BASIS_POINTS;

USDC.safeTransfer(liquidityPool, lpFee);
USDC.safeTransfer(treasury, treasuryFee);

Ensure that fees are calculated consistently for both position increases and decreases, preventing excess charges on referrer rewards and maintaining logical consistency across the contract.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The AutomationPositionsV2 and AutomationOrdersV2 contracts allow setting different PRICE_BAND_WIDTH values independently.

This inconsistency can lead to:

1. Mismatched Band Calculations: Orders and positions may be mapped to different bands, causing issues when deleting related orders tied to positions.

2. Missed Deletions: Orders may not be correctly identified and removed, potentially leading to orphaned entries or stale data.

Ensure synchronization of PRICE_BAND_WIDTH across related contracts.

1. Factory Contract: Deploy AutomationPositionsV2 and AutomationOrdersV2 through a factory contract that initializes them with the same PRICE_BAND_WIDTH value.

2. Immutable Band Width: Consider making PRICE_BAND_WIDTH immutable to prevent runtime changes that could lead to mismatches.

3. Shared Storage (Optional): Store the PRICE_BAND_WIDTH in a shared storage contract or config module to enforce consistency without duplicating values.

This approach guarantees synchronized band calculations, reduces the risk of mismatched deletions, and improves data consistency across contracts.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The getPerpetualPriceForPosition and getPerpetualPrice functions contain duplicate logic for calculating the perpetual price based on open interest (longOI and shortOI). This redundancy increases maintenance complexity and introduces a risk of inconsistent behavior if future changes are applied to only one of the functions.

Any adjustments to the perpetual price calculation logic would require updates in both functions, increasing the chance of errors or omissions.

Refactor the code to introduce a shared internal function that accepts longOI and shortOI as parameters. Both getPerpetualPriceForPosition and getPerpetualPrice should delegate their calculations to this new function.

This approach centralizes the perpetual price logic, simplifying future updates and ensuring consistent behavior across the contract.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The modifyPosition function calls getPerpetualPriceForPosition multiple times with the same input parameters, leading to redundant computations. These repeated calls increase gas costs and reduce efficiency.

Since the parameters remain unchanged during execution, the same result is recalculated unnecessarily instead of being reused.

Store the result of getPerpetualPriceForPosition in a temporary variable and reuse it throughout the function. For example:

uint256 perpetualPrice = getPerpetualPriceForPosition(key, indexPrice, int256(size), isLong);

Ensure that no parameter changes occur during execution that could affect the result. This optimization reduces gas consumption and improves performance without altering the logic.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The AutomationLiquidationsV2 contract uses a for loop to find and remove or update positions within priceBands during deletePosition and updatePosition. This approach is inefficient (O(n)) and can lead to higher gas costs and scalability issues as the number of positions grows.

Add an index field to the PositionA struct to track the position’s index within its price band. This allows O(1) operations instead of looping through all positions.

Update the logic to:

• Store the index when inserting a position.

• Directly access and replace positions by index when deleting or updating them.

This approach improves efficiency, reduces gas costs, and scales better with larger datasets.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The AutomationLiquidationsV2 contract contains duplicated logic between the following functions:

1. Position Deletion: _deletePosition and deletePosition perform almost identical tasks for removing positions from storage.

2. Liquidation Execution: _executePosLiquidations and executePosLiquidations duplicate the logic for processing liquidations.

This duplication increases code size, maintenance effort, and the risk of inconsistencies if updates are applied to one function but not the other.

Refactor the contract to centralize shared logic into internal helper functions. This approach reduces redundancy, improves maintainability, and ensures consistent behavior across similar operations.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The checkCallback function in Automation contracts contains repetitive code for verification and data processing. This duplication:

1. Increases Code Complexity: Makes it harder to read, maintain, and debug.

2. Slows Updates and Fixes: Any fixes or enhancements require changes in multiple places, increasing the risk of inconsistencies.

3. Logic Duplication Across Contracts: Contracts such as AutomationLiquidationsV2 and AutomationPositionsV2 share similar functionality but replicate logic instead of reusing code.

1. Extract Shared Logic into a Library or Internal Function:
   Move common logic (e.g., verification, price scaling, and report parsing) into a library or an internal utility function that can be reused across all contracts.

2. Unify Shared Components Across Contracts:
   Refactor shared logic between Automation contracts into base contracts or shared modules to avoid duplication and improve maintainability.

3. Enhance Readability and Testing:
   Simplify function implementation and improve test coverage by focusing on reusable components instead of repeatedly testing similar code blocks in different contracts.

This approach reduces redundancy, simplifies maintenance, and allows for faster updates and fixes while ensuring consistency across Automation contracts.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

In the checkCallback function, the line:

uint256[] memory positions = new uint256[](maxLength);

Allocates an array with a size of maxLength, which is calculated based on the number of positions across multiple bands. However, subsequent loops limit the j index to a maximum of 100 elements:

for (uint256 i; i < length && j < 100; ++i) {
    ...
}

This results in oversized memory allocation, wasting gas and memory space since only up to 100 positions are ever used.

Replace the array size initialization with a fixed value of 100:

uint256[] memory positions = new uint256[](100);

This ensures memory allocation matches the maximum allowed elements, improving gas efficiency without affecting functionality.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

Different contracts use inconsistent basis point scales:

• FeeManager: Uses BASIS_POINTS_X10 = 1e6 and BASIS_POINTS = 1e5.

• User: Uses REFERRAL_BASIS_POINTS = 1e10.

This inconsistency can:

1. Confuse Integrators and Frontend Developers: Requires explicit awareness of the scale used in each contract.

2. Cause Invalid Configurations: Incorrectly set values due to misunderstandings can lead to unexpected fee calculations and misconfigured discounts.

3. Increase Maintenance Overhead: Adding or updating logic requires additional checks to ensure compatibility across contracts.

Standardize all basis point definitions across contracts to use a single scale (e.g., 1e6). This simplifies integration, reduces configuration errors, and improves code consistency across the system.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.

The txType variable in the modifyPosition function of the FuturesCore contract is defined as a uint256 to represent transaction types (e.g., 0 for open, 1 for increase, 2 for decrease, and 3 for liquidation). Using plain integers can lead to ambiguity, misinterpretation, and errors during development and debugging, as the code does not inherently enforce valid values or provide meaningful context for each type.

Replace the txType variable with a typed enum to make the code more readable and type-safe. Enums enforce valid values and provide clear context, reducing the risk of passing invalid transaction types. Update all occurrences of txType assignments and comparisons accordingly to use the defined enum values.

ACKNOWLEDGED: The Dexodus team acknowledged this finding.