Skip to content

Latest commit

 

History

History
73 lines (64 loc) · 3.51 KB

File metadata and controls

73 lines (64 loc) · 3.51 KB

Dapper Raisin Locust

High

Attestation Reviews Not Transferred to New Ethos Profile Upon Reassignment

Vulnerability Details

Users can submit attestation reviews through EthosReview::addReview, which associates the review with the profile currently owning the attestation:

function addReview(
    Score score, address subject, address paymentToken, string calldata comment, string calldata metadata, AttestationDetails calldata attestationDetails
) external payable whenNotPaused {
    // --SNIP
  if (subject != address(0)) {// --SNIP}
  else {
      // convert the service/account to a hash as an identifier
      attestationHash = _getEthosAttestation().getServiceAndAccountHash(
        attestationDetails.service,
        attestationDetails.account
      );
>>>      mockId = ethosProfile.profileIdByAttestation(attestationHash);
>>>      mockId = _addReview(mockId, subject, true, attestationHash, ethosProfile);
    }
}

function _addReview( uint256 mockId, address subject, bool isAttestation, bytes32 attestationHash, IEthosProfile ethosProfile )
  internal returns (uint256 subjectProfileId) {
    // if profileId does not exist for subject, create and record a "mock"
    if (mockId == 0) {
      // audit-finding Not persisting reviews when registering the profile
      subjectProfileId = ethosProfile.incrementProfileCount(
        isAttestation,
        subject,
        attestationHash
      );
    } else {
      subjectProfileId = mockId;
    }

>>>    reviewIdsBySubjectProfileId[subjectProfileId].push(reviewCount);
  }

If an attestation is transferred to another Ethos profile via EthosAttestation::createAttestation, the profile ownership of the attestation is updated:

function _claimAttestation(uint256 profileId, bytes32 attestationHash, string calldata evidence) private returns (bool) {
  // --SNIP

  Attestation memory attestation = attestationByHash[attestationHash];
  if (attestation.profileId == profileId) {
      return false;
  }
  // --SNIP
  // Remove attestation from the previous profile
  _removeAttestationFromPreviousProfile(attestation.profileId, attestationHash);
  // Set new profileId for attestation
  attestationByHash[attestationHash].profileId = profileId;

  // Keep the profile contract up to date re: registered attestations
  IEthosProfile(ethosProfile).assignExistingProfileToAttestation(attestationHash, profileId);
}

The issue is that any reviews registered for the attestation under the previous profile are not transferred to the new profile that now owns the attestation. This is problematic because attestation reviews should be registered under whatever Ethos profile owning them

Proof Of Concept

Consider the following scenario:

  • Bob owns an X account linked to his Ethos profile (ID 3).
  • Three negative reviews are submitted for Bob’s X account, each associated with his profile.
  • Alice, who has Ethos profile ID 5, subsequently claims the X account by calling EthosAttestation::createAttestation. The account ownership is transferred to Alice’s profile (ID 5), but the three negative reviews remain associated with Bob’s profile rather than moving to Alice’s profile.

Impact

Reviews associated with an attestation do not persist when the attestation is moved to a new Ethos profile

Mitigation

Consider migrating attestation reviews when they are registered under new Ethos profile.