Bozho - When a mock address is registered to another user, it loses its reviews, votes, and comments

[sherlock-admin4]

Bozho

High

When a mock address is registered to another user, it loses its reviews, votes, and comments

Summary

In Ethos, each user has a profile identified by a unique id, that can have multiple addresses. Profiles can receive reviews, votes, attestations, and comments.

If an address doesn't belong to a profile, a new MOCK profile is created. Mock profiles track reviews, votes and comments made without an associated subject profileId. Each address is mapped to a profileId in the profileIdByAddress mapping, including mock profiles. When someone leaves a review for a mock profile, the review is mapped to the mock's profileId in the reviewIdsBySubjectProfileId mapping.

Eventually, if the the owner of the address attached to the mock profile has a actual actual profile, they can register the address (attached to the mock profile) to their actual profile by calling the registerAddress() function in EthosProfile.sol. The function updates the profileIdByAddress mapping to point to the actual profileId and remove the address from the mock profile.

373:  function registerAddress(address addressStr, uint256 profileId, uint256 randValue, bytes calldata signature)
        external
        whenNotPaused
        onlyNonZeroAddress(addressStr)
    {
       ...

        profiles[profileId].addresses.push(addressStr);
  📌    profileIdByAddress[addressStr] = profileId;

        ...
    }

EthosProfile.sol#404

Afterward, when we check the profileId for the address, it will return a profileId different from the mock profile's. This is the intended behavior, as the address is now part of the actual profile.

The problem arises in the EthosReview.sol contract, where reviewIdsBySubjectProfileId is not updated when the address is registered to the actual profile. This means that the review remains associated with the mock profile but not with the address, causing reviewIdsBySubjectAddress() to not functio as expected, leading to lost reviews.

  97:  function addReview(uint256 subjectProfileId, uint256 rating, string calldata review, uint256 randValue, bytes calldata signature)
        external
        whenNotPaused
        onlyNonZeroProfileId(subjectProfileId)
    {
        ...
  📌    reviewIdsBySubjectProfileId[subjectProfileId].push(reviewId);
        ...
    }

Same vulnerability also exists in the EthosDiscussion.sol and EthosVotes contracts, where replies[ethosProfile][targetId] and votesGeneralByContractByTargetId[ethosProfile][targetId] are also not updated when the address is registered to an actual profile.

This is important, beacuse reviews, discussions and votes are used for off-chain reputation score calculation. Losing them can lead to a loss of reputation.

Sources:

• Whitepaper

  Reviews provide the ability to develop a reputation outside of financially backed stakes.

• Another page of whitepaper

  The Credibility Score algorithm will factor in a variety of weighted indicators. Indicators include relevant on chain actions, such as:

  • average review rating and volume of reviews

  Under the hood

  The credibility score will be calculated using on chain Ethos protocol data only. The algorithm will be committed to the Ethos Smart Contract, with all details publicly verifiable.

• Sponsor in the public channel
  

• Sponsor confirming the issue in the private thread:
  

Root Cause

The following mappings are not updated when an address of a mock profile is registered to an actual profile:

• reviewIdsBySubjectProfileId[subjectProfileId]@EthosReview.sol
• replies[ethosProfile][targetId]@EthosDiscussion.sol
• votesGeneralByContractByTargetId[targetContract][targetId] @ EthosVote.sol

Internal pre-conditions

• Address is attached to a mock profile.
• Address is registered as a secondary address to an actual profile.

External pre-conditions

None

Attack Path

Scenario:

• 0x123 is an address with a mock profile.
• 0x456 is an address with an actual profile.
• Alice owns both addresses

---

1. Bob leaves a review for 0x123.
2. Bob also votes for 0x123.
3. Alice registers 0x123 to her actual profile.
4. Bob checks his reviews and votes for 0x123 and gets 0 results.

Impact

• reviewIdsBySubjectAddress() in EthosReview.sol
• hasVotedFor() and votesInRangeFor() in EthosVote.sol
• directRepliesInRange() and addReply() in EthosDiscussion.sol

will not return all reviews, votes, and comments associated with the address (only those specifically written for the verified profile before the registration).

Users will lose reviews and discussions associated with their mock profile upon registering the address to an actual profile, which will lead to incorrect off-chain reputation score calculations.

PoC

Place the following code in address.register.test.ts:

Click to see the code

// eslint-disable-next-line jest/no-focused-tests
it.only('should lose reviews when a mock profile’s address is registered to another user', async () => {
  const ethosReview: EthosReview = deployer.ethosReview.contract;
  const userAAddress2 = await deployer.newWallet();

  await userB.review({ address: userAAddress2.address });
  // Verify the review was created
  const userAAddress2ProfileId = await ethosProfile.profileIdByAddress(userAAddress2.address);
  const reviews = await ethosReview.reviewsBySubjectInRange(userAAddress2ProfileId, 0, 10);
  // userAAddress2 should own 1 review
  expect(reviews.length).to.equal(1);
  expect(reviews[0][3]).to.equal(userAAddress2.address); // Review.subject

  // at this point userAAddress2 should be mapped to a mock profile
  const userAAddress2Status = await ethosProfile.profileStatusById(userAAddress2ProfileId);
  expect(userAAddress2Status[2]).to.equal(true); // Mock == true
  expect(userAAddress2Status[0]).to.equal(false); // Verified == false

  // Register userAAddress2 to userA
  await registerAddress(userA, userAAddress2.address);

  // verify userAAddress2 is now part of userA's profile
  const userAAddresses = await ethosProfile.addressesForProfile(userA.profileId);
  expect(userAAddresses.length).to.equal(2);
  expect(userAAddresses[0]).to.equal(userA.signer.address);
  expect(userAAddresses[1]).to.equal(userAAddress2.address);

  const userAAddress2ProfileIdNEW = await ethosProfile.profileIdByAddress(userAAddress2.address);
  // verify the review is no longer associated with userAAddress2
  const reviewsAfterRegistration = await ethosReview.reviewsBySubjectInRange(
    userAAddress2ProfileIdNEW,
    0,
    10,
  );

  // review still exists but is no longer associated with userAAddress2 as
  // it is mapped to the old profileId of userAAddress2 and doesn't get updated
  expect(reviewsAfterRegistration.length).to.equal(0);
});

Mitigation

Update the profileId in the reviewIdsBySubjectProfileId, votesGeneralByContractByTargetId and replies mappings when the address is registered to an actual profile using registerAddress().

⚠ Be cautious when updating these mappings, as a user may write a review for a mock profile before registering the address to their actual profile, which would cause the user to review themselves.