024.md

Main Gauze Kangaroo

Medium

ReputationMarket.sol :: Users can sell all their votes for a specific profileId but still retain isParticipant = true.

Summary

isParticipant is a mapping that checks if a user has votes for a specific profileId. It returns true if the user has votes and false otherwise.

The issue arises when a user sells all their votes for a particular profileId. In this case, isParticipant is not updated to false, causing the mapping to incorrectly indicate that the user still has votes for that profileId, even though they do not.

Root Cause

When a user purchases votes for a specific profileId using the buyVotes() for the first time, the isParticipant mapping is set to true, and the user is added to the participant array.

function buyVotes(
    uint256 profileId,
    bool isPositive,
    uint256 maxVotesToBuy,
    uint256 minVotesToBuy
  ) public payable whenNotPaused activeMarket(profileId) nonReentrant {

///code

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

///code 
}

If we review the participant array, we find the following.

// profileId => participant address
  // append only; don't bother removing. Use isParticipant to check if they've sold all their votes.
  mapping(uint256 => address[]) public participants;

As you can see when the users sells all their votes for a specific profileId is not removed for the array but the isParticpant mapping needs to be set to false to know that the user has no votes for this proileId.

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

There is no mechanism to check if a user has sold all their votes for a specific profileId and update the isParticipant to false. This causes a situation where a user can sell all their votes, but their isParticipant status remains true.

Internal Pre-conditions

None.

External Pre-conditions

The user needs to sell all their votes for a specific profileId.

Attack Path

None.

Impact

A user with any votes for a specific profileId will be considered a participant.

PoC

To observe the issue, copy the following proof of concept into rep.market.test.ts.

it('The user retains their status as a participant unless they no longer have votes associated with this profileId.', async () => {
    
    //User has no votes for this profileId -> isParticipant = false
    expect(
      await reputationMarket.isParticipant(DEFAULT.profileId, await userA.signer.getAddress()),
    ).to.equal(false);
    
    // buy one positive vote
     await userA.buyOneVote();

     expect(
      await reputationMarket.isParticipant(DEFAULT.profileId, await userA.signer.getAddress()),
    ).to.equal(true);

    // sell one positive vote
    await userA.sellOneVote();

    //no votes for this profileId
    const { trustVotes, distrustVotes } = await userA.getVotes();
    expect(trustVotes).to.equal(0);
    expect(distrustVotes).to.equal(0);

     //The user retains their status as a participant unless they no longer have votes associated with this profileId
     expect(
      await reputationMarket.isParticipant(DEFAULT.profileId, await userA.signer.getAddress()),
    ).to.equal(true);
  });

Mitigation

To resolve the issue in sellVotes(), check if the user has no votes remaining for the profileId, and if so, set isParticipant to false.

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;

+   MarketInfo memory userInfo = getUserVotes(msg.sender, profileId);
+    if(userInfo.trustVotes == 0 && userInfo.distrustVotes == 0) {
+      isParticipant[profileId][msg.sender] = false;
+    }

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