056.md

Slow Tan Swallow

High

buyVotes makes ReputationMarket insolvent

Summary

Users use buyVotes to vote for another user and for such they pay for every vote in ETH.

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L442

  function buyVotes(...) public payable whenNotPaused activeMarket(profileId) nonReentrant {
    _checkMarketExists(profileId);

    // Determine how many votes can be bought with the funds provided
    (
      uint256 votesBought,
      uint256 fundsPaid,
      ,
      uint256 protocolFee,
      uint256 donation,
      uint256 minVotePrice,
      uint256 maxVotePrice
    ) = _calculateBuy(markets[profileId], isPositive, msg.value);

The price, votes and the amount this user would pay is calculated inside _calculateBuy, where we would take note that returned fundsPaid is all of the ETH used to buy votes + both protocolFee and donation fees

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L978

    fundsPaid += protocolFee + donation;

    maxPrice = votePrice;

    return (votesBought, fundsPaid, votePrice, protocolFee, donation, minPrice, maxPrice);
  }

Later when we get back into buyVotes we can see that an issue appears. We send the appropriate fees using applyFees(protocolFee, donation, profileId);, however at the end of the function we also increase marketFunds[profileId] by fundsPaid. From _calculateBuy we have seen that fundsPaid already accounts protocolFee and donation, but these are already sent, meaning they they are accounted twice! Once send in applyFees and then added to marketFunds

https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L481

    _checkSlippageLimit(votesBought, expectedVotes, slippageBasisPoints);

    // Send fees
    applyFees(protocolFee, donation, profileId);

    markets[profileId].votes[isPositive ? TRUST : DISTRUST] += votesBought;
    votesOwned[msg.sender][profileId].votes[isPositive ? TRUST : DISTRUST] += votesBought;

    if (!isParticipant[profileId][msg.sender]) {
      participants[profileId].push(msg.sender);
      isParticipant[profileId][msg.sender] = true;
    }

    uint256 refund = msg.value - fundsPaid;
    if (refund > 0) _sendEth(refund);
    
    // Buy increase `marketFunds` by `fundsPaid +  protocolFee + donation` fees ?
    marketFunds[profileId] += fundsPaid;

Root Cause

marketFunds being increased by protocolFee and donation, instead of only the real votes value.

marketFunds[profileId] += fundsPaid;

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Insolvency since the contract holds all ETH balances, meaning if we send the fees and later withdrawGraduatedMarketFunds is called, we would have taken more ETH out than we attributed inside the internal accounting. This ETH will come from other profile votes, eventually leading to insolvency.

PoC

No response

Mitigation

Consider these 2 changes

Inside _calculateBuy:

    while (fundsAvailable >= votePrice) {
      fundsAvailable -= votePrice;
      fundsPaid += votePrice;
      votesBought++;

      market.votes[isPositive ? TRUST : DISTRUST] += 1;
      votePrice = _calcVotePrice(market, isPositive);
    }
    // In order for `fundsPaid` to be only the value paid for buying votes
-   fundsPaid += protocolFee + donation;

    maxPrice = votePrice;

Inside buyVotes:

-   uint256 refund = msg.value - fundsPaid;
+   uint256 refund = msg.value - (fundsPaid +donation + protocolFee) ;
    if (refund > 0) _sendEth(refund);

    emit VotesBought(
      profileId,
      msg.sender,
      isPositive,
      votesBought,
-     fundsPaid
+     fundsPaid + donation + protocolFee, // So the event is emitted by the old standard (all funds used)
      block.timestamp,
      minVotePrice,
      maxVotePrice
    );

This way the amounts will be calculated correctly, while at the same time the user will be refunded what he is supposed to.