088.md

Shambolic Turquoise Panda

High

ReputationMarket.sellVotes doesnt correctly check for slippage

Summary

In ReputationMarket.sellVotes slippage can be defined by the user by including a minimumVotePrice. However slippage is not correctly checked as fees are still retained and used to check for slippage determined by the user via minimumVotePrice. This can cause unintended loss for the user.

  /**
   * @notice Sell trust or distrust votes from a market
   * @dev Protocol fees are taken from the sale proceeds.
   *      Proceeds are sent to the seller after fees.
   * @param profileId The ID of the market to sell votes in
   * @param isPositive True to sell trust votes, false to sell distrust votes
   * @param votesToSell Number of votes to sell
   * @param minimumVotePrice Minimum acceptable price per vote (protects against slippage)
   */
  function sellVotes(
    uint256 profileId,
    bool isPositive,
    uint256 votesToSell,
    uint256 minimumVotePrice
  ) public whenNotPaused activeMarket(profileId) nonReentrant {
    _checkMarketExists(profileId);
    (uint256 proceedsBeforeFees, uint256 protocolFee, uint256 proceedsAfterFees) = _calculateSell(
      markets[profileId],
      profileId,
      isPositive,
      votesToSell
    );

@>    uint256 pricePerVote = votesToSell > 0 ? proceedsBeforeFees / votesToSell : 0; // @audit pricePerVote calculated before fees are deducted
@>    if (pricePerVote < minimumVotePrice) { // @audit slippage check with no fees deducted
      revert SellSlippageLimitExceeded(minimumVotePrice, pricePerVote);
    }

    markets[profileId].votes[isPositive ? TRUST : DISTRUST] -= votesToSell;
    votesOwned[msg.sender][profileId].votes[isPositive ? TRUST : DISTRUST] -= votesToSell;
    // tally market funds
    marketFunds[profileId] -= proceedsBeforeFees;

    // apply protocol fees
    applyFees(protocolFee, 0, profileId);

    // send the proceeds to the seller
@>    _sendEth(proceedsAfterFees); // @audit proceeds send to user with fee deduction

    emit VotesSold(
      profileId,
      msg.sender,
      isPositive,
      votesToSell,
      proceedsAfterFees,
      block.timestamp
    );
    _emitMarketUpdate(profileId);
  }

Root Cause

The code checks if the price per vote meets the user's minimum price requirement before deducting protocol fees, rather than after. This means the actual amount received can be lower than expected.

Attack Path

1. User sells 1 trust vote and sets minVotePrice=1 ETH to expect at least 1 ETH in return. Suppose protocolFee is 5%.
2. Price per trust vote is calculated as 1 ETH
3. After fee deduction, user receives 0.95 ETH and protocol receives 0.05 ETH in fees
4. At the end, user receives less funds that expected (0.95 ETH < 1 ETH)

While users could work around this by setting a higher minimum price (like 1.1 ETH instead of 1 ETH), this is not a good solution. It requires users to read the codebase to understand the internal fee calculations and manually adjust their minimum price upward. Even if a seller sets a reasonable minimumVotePrice 1 ETH, proceeds of at least 1 ETH is not guaranteed to be send to them.

LOC

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

Impact

Unintended loss of funds for user

Mitigation

Check for slippage after deducting protocolFees via proceedsAfterFees

  function sellVotes(
    uint256 profileId,
    bool isPositive,
    uint256 votesToSell,
    uint256 minimumVotePrice
  ) public whenNotPaused activeMarket(profileId) nonReentrant {
    _checkMarketExists(profileId);
    (uint256 proceedsBeforeFees, uint256 protocolFee, uint256 proceedsAfterFees) = _calculateSell(
      markets[profileId],
      profileId,
      isPositive,
      votesToSell
    );

-    uint256 pricePerVote = votesToSell > 0 ? proceedsBeforeFees / votesToSell : 0;
+    uint256 pricePerVote = votesToSell > 0 ? proceedsAfterFees / votesToSell : 0;
    if (pricePerVote < minimumVotePrice) {
      revert SellSlippageLimitExceeded(minimumVotePrice, pricePerVote);
    }

    markets[profileId].votes[isPositive ? TRUST : DISTRUST] -= votesToSell;
    votesOwned[msg.sender][profileId].votes[isPositive ? TRUST : DISTRUST] -= votesToSell;
    // tally market funds
    marketFunds[profileId] -= proceedsBeforeFees;

    // apply protocol fees
    applyFees(protocolFee, 0, profileId);

    // send the proceeds to the seller
    _sendEth(proceedsAfterFees);

    emit VotesSold(
      profileId,
      msg.sender,
      isPositive,
      votesToSell,
      proceedsAfterFees,
      block.timestamp
    );
    _emitMarketUpdate(profileId);
  }