Skip to content

Latest commit

 

History

History
75 lines (50 loc) · 2.92 KB

030.md

File metadata and controls

75 lines (50 loc) · 2.92 KB

Tangy Tortilla Fox

Medium

Admin market removals will make users chose the wrong market

Summary

All available markets are in an array of marketConfigs. Where each index has it's own unique config and users can have only 1 market with only 1 config.

Admins can addMarketConfig and removeMarketConfig, however when doing an add the place of the last index would replace the current removed index passably causing a user to pick the wrong index if the change and market creation happens at the same time.

Root Cause

_createMarket relying on indexes without any ''slippage/verification" check

https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L318

  function _createMarket(
    uint256 profileId,
    address recipient,
    uint256 marketConfigIndex
  ) private nonReentrant {
    // ensure a market doesn't already exist for this profile
    if (markets[profileId].votes[TRUST] != 0 || markets[profileId].votes[DISTRUST] != 0)
      revert MarketAlreadyExists(profileId);

    // ensure the specified config option is valid
    if (marketConfigIndex >= marketConfigs.length)
      revert InvalidMarketConfigOption("Invalid config index");

    uint256 creationCost = marketConfigs[marketConfigIndex].creationCost;

    // Handle creation cost, refunds and market funds for non-admin users
    if (!hasRole(ADMIN_ROLE, msg.sender)) {
      if (msg.value < creationCost) revert InsufficientLiquidity(creationCost);
      marketFunds[profileId] = creationCost;
      if (msg.value > creationCost) {
        _sendEth(msg.value - creationCost);
      }
    } else {
      // when an admin creates a market, there is no minimum creation cost; use whatever they sent
      marketFunds[profileId] = msg.value;
    }

Internal Pre-conditions

  1. User wants to create a luxurious market (good params + high creationCost)

External Pre-conditions

  1. Admin removes an index

Attack Path

  1. Famous user wants to create a deep market with lots of liquidity (it will be expensive)
  2. He selects the 3rd index and calls createMarketWithConfig
  3. At the same time an admin removes this config and removeMarketConfig replaces it with the last
  4. The user TX executes second and his market is made with a cheap config (low creationCost) and bad parameters

Our user can only create 1 market and wasting this opportunity on the wrong config will mean that he would not use it.

Impact

Users have only 1 market per ID, i.e. 1 market per actual user. Wasting this market on the wrong config would mean that this user will not be a part of ethos.

PoC

No response

Mitigation

While adding market push them to the array and while removing them instead of deleting each market and changing the array order, just mark it as un-usable and leave it there. This way the user TX would revert and he would be able to create a market with another config of his choice.