009.md

Main Gauze Kangaroo

Medium

ReputationMarket.sol :: sellVotes() may result in users receiving less than the minimumVotePrice per vote, leading to a potential loss of funds.

Summary

The sellVotes() is designed to sell votes associated with a specific profileId, with a minimumVotePrice to ensure votes aren't sold below a certain price.

However, the issue lies in the fact that the check for minimumVotePrice is performed before applying the fees. This causes users to unknowingly sell their votes for less than the minimumVotePrice receiving less amount for their votes.

Root Cause

sellVotes() is implemented as follows.

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;
    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);
  }

As you can see, the minimumVotePrice check is performed using proceedsBeforeFees instead of proceedsAfterFees, which represents the actual amount the user will receive. This discrepancy becomes more evident when examining the previewExitFees(), which is called within _calculateSell().

function previewExitFees(
    uint256 proceedsBeforeFees
  ) private view returns (uint256 totalProceedsAfterFees, uint256 protocolFee) {
    protocolFee = (proceedsBeforeFees * exitProtocolFeeBasisPoints) / BASIS_POINTS_BASE;
    totalProceedsAfterFees = proceedsBeforeFees - protocolFee;
  }

As you can see, the fees are subtracted, which means that proceedsBeforeFees will be greater than proceedsAfterFees. This results in users potentially receiving less than the minimumVotePrice per vote.

Internal Pre-conditions

None.

External Pre-conditions

The user invokes sellVotes() to sell their votes.

Attack Path

None.

Impact

Users may receive less than the specified minimumVotePrice, leading to a potential loss of funds.

PoC

To illustrate the issue, let's consider the following example:

A user wants to sell 10 votes with a minimumVotePrice = 95, and the exitProtocolFeeBasisPoints = 1000(10%). Assume the profile exits and the price per vote is 100.

1. The user calls sellVotes with votesToSell = 10 and minimumVotePrice = 95.

2. _calculateSell() returns:

   • proceedsBeforeFees = 1000 (10 * 100)
   • protocolFee = 100 (10% of 1000)
   • proceedsAfterFees = 900 (proceedsBeforeFees - protocolFee).

3. The pricePerVote is calculated as:
   pricePerVote = proceedsBeforeFees / votesToSell = 1000 / 10 = 100.

4. The minimumVotePrice check is performed and passes because:
   pricePerVote (100) >= minimumVotePrice (95) -> true.

5. However, the user ultimately receives proceedsAfterFees = 900, which equates to a pricePerVote of:
   900 / 10 = 90.

This results in the user receiving less than the intended minimumVotePrice of 95 per vote, causing a loss of funds. Specifically, the user expected minimum 950 tokens (95 * 10 votes) but only received 900 tokens, resulting in a loss of 50 tokens.

Mitigation

To resolve the issue, use proceedsAfterFees to calculate the pricePerVote instead of proceedsBeforeFees.

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);
  }