Skip to content

Latest commit

 

History

History
68 lines (43 loc) · 3.51 KB

File metadata and controls

68 lines (43 loc) · 3.51 KB

Mean Brick Mongoose

Medium

Malicious validator can force users to create a different market than they intended

Summary

the removeMarketConfig function allows an admin to remove a market. If the market is not the last index, it is swapped with the last index market and then removed. The problem occurs when a malicious validator can backrun a user's tx in order for the user to create a market he did not intend.

Root Cause

In ReputationMarket.sol ln 388 https://github.com/sherlock-audit/2024-12-ethos-update/blob/c3a2b007d0ddfcb476f300f8b766808f0e3e2dfd/ethos/packages/contracts/contracts/ReputationMarket.sol#L388

  function removeMarketConfig(uint256 configIndex) public onlyAdmin whenNotPaused {
    // 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();
  }

The admin can remove a market, this is done by swapping the index with the last index and popping the last index. The problem occurs when a malicious validator sees a user who is attempting to create a market which is also going to be removed in the same block by the admin. In normal execution the user would have created the market using the config, then the admin tx would be executed and the market config index would be deleted from the list. However a malicious validator can reorganize the block and back run the user's transaction to occur right after the admins removal transaction. In this scenario 2 thing could happen. The market created has a lower creation cost and the user is refunded the excess eth. Or the user sent extra eth to be safe and the eth is consumed by a larger creation cost market.

In each scenario the user is forced to take an action unwanted by him. Because the function removeMarketConfig has the whenNotPaused modifier, it must be called during the unpaused state which causes this issue.

Internal Pre-conditions

  1. normal user must attempt to create a market which has a market config index that is not the last one in the list
  2. admin must have also submit a transaction in the same block to remove said index.

External Pre-conditions

No response

Attack Path

  1. malicious validator sees in the mempool a user attempting to create a market with market config index that is not the last market config
  2. Malicious validator also sees on the mempool a transaction to remove the same market config.
  3. malicious user backruns the users tx to come after the admin tx
  4. User is forced to pay and create a market he did not desire.

Impact

User is forced to create a market he did not desire. There is 2 outcomes, 1 the different market config features a lower creation cost and he is refunded the excess eth. Or the market config features a more expensive creation cost and the user is charged a higher amount which would be taken from him in the chance he sent excess eth. The second scenario could be considered user error but the first scenario is not.

PoC

No response

Mitigation

Only allow market configs to be deleted during the paused state.