005.md

Loud Onyx Perch

High

Wrong slippage protection when selling votes

Summary

When sellers sell their votes, they use the sellVotes function, the protocol also provides a way to protect against sandwich attacks, by allowing them to pass a slippage protection, minimumVotePrice.

  function sellVotes(
    uint256 profileId,
    bool isPositive,
    uint256 votesToSell,
@>  uint256 minimumVotePrice
  ) public whenNotPaused activeMarket(profileId) nonReentrant {
    // ...
  }

Later, the price per vote is calculated and compared to the provided minimumVotePrice, however, the issue here is that the protocol is using proceedsBeforeFees to do so:

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

This is wrong, as proceedsBeforeFees is an exaggerated amount of proceedsAfterFees, which contains the exit fees and doesn't properly reflect the resulting vote price the user will receive.

NB: Even if the seller is aware of the fees, there's still a possibility that the exit fees change just before the execution of a sellVotes transaction, leaving it vulnerable to sandwich attacks.

Root Cause

ReputationMarket::sellVotes uses the proceedsBeforeFees when calculating the price per vote, which is wrong as it doesn't account for fees, https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L553.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

Slippage protection will not work as expected, forcing sellers to lose funds.

PoC

Add the following in ethos/packages/contracts/test/reputationMarket/rep.fees.test.ts:

describe('PoC', () => {
  let user1: HardhatEthersSigner, user2: HardhatEthersSigner;

  beforeEach(async () => {
    user1 = ethosUserA.signer;
    user2 = await deployer.createUser().then(async (ethosUser) => {
      await ethosUser.setBalance('2000');

      return ethosUser.signer;
    });

    await reputationMarket.connect(deployer.ADMIN).setExitProtocolFeeBasisPoints(5_00);
  });

  it('sellVotes - wrong slippage protection', async () => {
    // User 1 buys 1 vote
    await reputationMarket
      .connect(user1)
      .buyVotes(DEFAULT.profileId, true, 1, 1, { value: DEFAULT.creationCost });

    // User 2 sells 1 vote
    await reputationMarket
      .connect(user2)
      .buyVotes(DEFAULT.profileId, true, 1, 1, { value: DEFAULT.creationCost });

    const expectedVotePrice = ethers.parseEther('0.005');
    const balanceBefore = await ethers.provider.getBalance(user1.address);

    // User 1 sells 1 vote and expects to receive >= 0.005 ETH
    await reputationMarket
      .connect(user1)
      .sellVotes(DEFAULT.profileId, true, 1, expectedVotePrice);

    const balanceAfter = await ethers.provider.getBalance(user1.address);

    // Received less than expected
    expect(balanceAfter - balanceBefore).to.be.lessThan(expectedVotePrice);
  });
});

Mitigation

Use the proceedsAfterFees instead of proceedsBeforeFees when accounting for 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;
+   uint256 pricePerVote = votesToSell > 0 ? proceedsAfterFees / votesToSell : 0;
    if (pricePerVote < minimumVotePrice) {
      revert SellSlippageLimitExceeded(minimumVotePrice, pricePerVote);
    }

    // ...
  }