Skip to content

Latest commit

 

History

History
94 lines (67 loc) · 3.26 KB

006.md

File metadata and controls

94 lines (67 loc) · 3.26 KB

Loud Onyx Perch

Medium

isParticipant is not cleared when selling all votes

Summary

When a user buys some votes, he's added to the participants array and isParticipant map, this indicates that the user holds some votes for that market.

// Add buyer to participants if not already a participant
if (!isParticipant[profileId][msg.sender]) {
  participants[profileId].push(msg.sender);
  isParticipant[profileId][msg.sender] = true;
}

When the user sells his votes he is expected to be removed from the map but remains in the participants' arrays, this could be confirmed by:

  1. "append only; don't bother removing. Use isParticipant to check if they've sold all their votes.", https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L131-L133.
  2. "@\return The number of historical participants (includes addresses that sold all votes)", https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L782-L790.

However, this does not happen when the user sells all his votes, and isParticipant will keep showing true.

Root Cause

isParticipant is not cleared after selling all the user's votes, https://github.com/sherlock-audit/2024-12-ethos-update/blob/main/ethos/packages/contracts/contracts/ReputationMarket.sol#L539-L578.

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

isParticipant is not cleared after selling all the user's votes, which still indicates that he holds some votes, even though he doesn't.

PoC

Add the following test 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);
    await reputationMarket.connect(deployer.ADMIN).setEntryProtocolFeeBasisPoints(5_00);
    await reputationMarket.connect(deployer.ADMIN).setDonationBasisPoints(5_00);
  });

  it('isParticipant not clearing when selling votes', async () => {
    // User 1 buys 1 vote
    await reputationMarket
      .connect(user1)
      .buyVotes(DEFAULT.profileId, true, 1, 1, { value: DEFAULT.creationCost });

    // User 1 is added as a participant
    expect(await reputationMarket.isParticipant(DEFAULT.profileId, user1.address)).to.be.equal(
      true,
    );

    // User 1 sells 1 vote
    await reputationMarket.connect(user1).sellVotes(DEFAULT.profileId, true, 1, 0);

    // User 1 is still a participant
    expect(await reputationMarket.isParticipant(DEFAULT.profileId, user1.address)).to.be.equal(
      true,
    );
  });
});

Mitigation

Clear isParticipant when a seller sells all his votes, by adding something like the following in sellVotes:

if (votesOwned[msg.sender][profileId].votes[isPositive ? TRUST : DISTRUST] == 0) {
  // remove the participant if they've sold all their votes
  isParticipant[profileId][msg.sender] = false;
}