073.md

Calm Fiery Llama

High

Unclaimable rewards for verified mock profiles due to rewards being tied to the mock's profileId

Summary

Vouchers pay a donation fee to the subject they vouch for, if the entryDonationFeeBasisPoints is greater than 0. Additionally, vouching for mock profiles is allowed in case they are later verified (i.e., registered to an existing profile).

However, since the rewards are tied to the profileId of the subject, if mock profiles are verified, their rewards will not be claimable because their profileId will change to that of the profile registering them.

Root Cause

In EthosVouch.sol, rewards from donation fees are tied to the subject's profile Id instead of a specific address of that profile.

Internal pre-conditions

1. The entryDonationFeeBasisPoints must be greater than 0.
2. A mock needs to be active. This occurs, for example, when a review is added to an unregistered address.

External pre-conditions

None.

Attack Path

1. A user vouches for a mock by calling EthosVouch::vouchByAddress(). The rewards are tied to the mock's current ProfileId.
2. Another user calls EthosProfile::registerAddress() to register the mock to their profile. The mock's ProfileId is now the one of the profile that registered it.
3. The original mock calls EthosVouch::claimRewards(), but they can only claim the rewards associated with their new ProfileId. This call reverts if the ProfileId does not have any claimable rewards.

Impact

A mock cannot claim their rewards if it has been verified with a profile after it has been vouched for. The rewards will be stuck in the contract forever.

PoC

The following needs to be added in EthosVouch.test.ts:

import { type HardhatEthersSigner } from '@nomicfoundation/hardhat-ethers/signers.js';
import { type EthosReview } from '../typechain-types/index.js';
import { common } from './utils/common.js';

const Score = {
  Negative: 0,
  Neutral: 1,
  Positive: 2,
};

type AttestationDetails = {
  account: string;
  service: string;
};
const defaultComment = 'default comment';
const defaultMetadata = JSON.stringify({ itemKey: 'item value' });

async function allowPaymentToken(
  admin: HardhatEthersSigner,
  ethosReview: EthosReview,
  paymentTokenAddress: string,
  isAllowed: boolean,
  priceEth: bigint,
): Promise<void> {
  await ethosReview.connect(admin).setReviewPrice(isAllowed, paymentTokenAddress, priceEth);
}

Now, the following test can be added in EthosVouch.test.ts:

it('should fail if rewards for claimed mock', async () => {
  const {
    ethosProfile,
    ethosReview,
    ethosVouch,
    ADMIN,
    OWNER,
    PROFILE_CREATOR_0,
    PROFILE_CREATOR_1,
    OTHER_0,
    EXPECTED_SIGNER,
    VOUCHER_0,
  } = await loadFixture(deployFixture);
  
  await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address);
  await ethosProfile.connect(PROFILE_CREATOR_0).createProfile(1);
  
  const reviewPrice = ethers.parseEther('1.23456789');
  await allowPaymentToken(ADMIN, ethosReview, ethers.ZeroAddress, true, reviewPrice);
  
  const params = {
    score: Score.Positive,
    subject: OTHER_0.address,
    paymentToken: ethers.ZeroAddress,
    comment: defaultComment,
    metadata: defaultMetadata,
    attestationDetails: {
      account: '',
      service: '',
    } satisfies AttestationDetails,
  };
  
  await ethosReview
    .connect(PROFILE_CREATOR_0)
    .addReview(
      params.score,
      params.subject,
      params.paymentToken,
      params.comment,
      params.metadata,
      params.attestationDetails,
      { value: reviewPrice },
    );
  
  await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_1.address);
  await ethosProfile.connect(PROFILE_CREATOR_1).createProfile(1);

  await ethosProfile.connect(OWNER).inviteAddress(VOUCHER_0.address);
  await ethosProfile.connect(VOUCHER_0).createProfile(1);
  
  expect(await ethosProfile.profileIdByAddress(OTHER_0.address)).to.be.equal(
    3,
    'wrong mock Id',
 );

  // set donation fee to 1000
  await ethosVouch.connect(ADMIN).setEntryDonationFeeBasisPoints(1000);

  expect(await ethosVouch.entryDonationFeeBasisPoints()).to.be.equal(
    1000,
    'wrong donation fee',
  );
        
  await ethosVouch.connect(VOUCHER_0).vouchByProfileId(3, DEFAULT_COMMENT, DEFAULT_METADATA, {
    value: ethers.parseEther('1.1'),
  });

  expect(await ethosVouch.vouchCount()).to.equal(1, 'Wrong vouchCount');
        
  // rewards should be 0.1 eth
  expect(await ethosVouch.rewards(3)).to.be.equal(
    100000000000000000n,
    'wrong rewards',
  );

  // mock is registered with a profile
  const signature = await common.signatureForRegisterAddress(
    OTHER_0.address,
    '4',
    '29548234957',
    EXPECTED_SIGNER,
  );
        
  await ethosProfile
    .connect(PROFILE_CREATOR_1)
    .registerAddress(OTHER_0.address, 4, 29548234957, signature);
  
  // profile Id for mock changes
  expect(await ethosProfile.profileIdByAddress(OTHER_0.address)).to.be.equal(
    4,
    'wrong profileId',
  );

  // new profile Id has its own rewards, in this case 0
  expect(await ethosVouch.rewards(4)).to.be.equal(
    0,
    'wrong rewards',
  );

  // rewards of old profile Id cannot be claimed, instead it reverts as the new one has 0 rewards
  await expect(
    ethosVouch
      .connect(OTHER_0)
      .claimRewards(),
  ).to.be.revertedWithCustomError(ethosVouch, 'InsufficientRewardsBalance');
});

Mitigation

Consider tying the rewards for a subject that is a mock to its address instead of its ProfileId.