Quaint Mulberry Mustang - Market Configuration Index Inconsistency

[sherlock-admin2]

Quaint Mulberry Mustang

Medium

Market Configuration Index Inconsistency

Summary

The removeMarketConfig function introduces an inconsistency by swapping the last configuration in the array with the one being removed. This behavior disrupts the expected indexing of configuration parameters, leading to the creation of markets with unexpected settings when users rely on specific indices.

Root Cause

When a configuration is removed, the function replaces the targeted index with the configuration at the end of the array and then removes the last element:
https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/57c02df7c56f0b18c681a89ebccc28c86c72d8d8/ethos/packages/contracts/contracts/ReputationMarket.sol#L403-L406

function removeMarketConfig(uint256 configIndex) public onlyAdmin whenNotPaused {//checked
    // Cannot remove if only one config remains
    if (marketConfigs.length <= 1) {
      revert InvalidMarketConfigOption("Must keep one config");
    }

    // Check if the index is valid
    if (configIndex >= marketConfigs.length) {
      revert InvalidMarketConfigOption("index not found");
    }

    emit MarketConfigRemoved(configIndex, marketConfigs[configIndex]);

    // If this is not the last element, swap with the last element
    uint256 lastIndex = marketConfigs.length - 1;
    if (configIndex != lastIndex) {
@>    marketConfigs[configIndex] = marketConfigs[lastIndex];
    }
     // Remove the last element
@>  marketConfigs.pop();
  }

This index swap results in configurations being reordered, breaking the correspondence between indices and their original parameter sets. Users interacting with createMarketWithConfig(configIndex) may unintentionally create markets using unexpected configurations.

Internal pre-conditions

N/A

External pre-conditions

No response

Attack Path

1. There are 3 configs
2. Admin removes config at index 1
3. user create market with configIndex=1

Impact

Markets could be created with unintended initial parameters

PoC

N/A

Mitigation

To address this issue, avoid swapping configurations when removing an entry. Instead:

Use an ordered deletion mechanism that retains the array's structure.