sherlock-audit / 2024-10-ethos-network-judging

0 stars 0 forks source link

LeFy - A malicious compromised address of a profile can claim malicious attestations to a profile #245

Open sherlock-admin3 opened 3 weeks ago

sherlock-admin3 commented 3 weeks ago

LeFy

Medium

A malicious compromised address of a profile can claim malicious attestations to a profile

Summary

In 'EthosProfile.sol' a user can delete an address thats compromised/hacked and it will be set as compromised, but that compromised address can still claim attestations in 'EthosAttestation.sol' and add malicious attestations to the profile and further ruin the profile's reputation.

Root Cause

In 'EthosAttestation.sol' there is no check that ensures that the msg.sender is not a compromised address of the profile:

  function createAttestation(
    uint256 profileId,
    uint256 randValue,
    AttestationDetails calldata attestationDetails,
    string calldata evidence,
    bytes calldata signature
  ) external whenNotPaused {
    validateAndSaveSignature(
      _keccakForCreateAttestation(
        profileId,
        randValue,
        attestationDetails.account,
        attestationDetails.service,
        evidence
      ),
      signature
    );

    bytes32 hashStr = getServiceAndAccountHash(
      attestationDetails.service,
      attestationDetails.account
    );

    bool isClaimed = _claimAttestation(profileId, hashStr, evidence);
    if (isClaimed) {
      return;
    }

    bool isRestore = restoreIfArchived(hashStr);
    if (isRestore) {
      return;
    }

    _attestationShouldNotExist(hashStr);

    address ethosProfile = _getEthosProfile();

    // ensure specified profile is active
    (bool profileExists, ) = ITargetStatus(ethosProfile).targetExistsAndAllowedForId(profileId);
    if (!profileExists) {
      revert ProfileNotFound(profileId);
    }

    // ensure profile exists for sender address
    uint256 verifiedProfileId = IEthosProfile(ethosProfile).verifiedProfileIdForAddress(msg.sender);
    // ensure the requested attestation profile is the same as the sender's verified profile
    if (verifiedProfileId != profileId) {
      revert AddressNotInProfile(msg.sender, profileId);
    }

    attestationHashesByProfileId[profileId].push(hashStr);
    hashIndexByProfileIdAndHash[profileId][hashStr] =
      attestationHashesByProfileId[profileId].length -
      1;

    attestationByHash[hashStr] = Attestation({
      archived: false,
      attestationId: attestationCount,
      createdAt: block.timestamp,
      profileId: profileId,
      account: attestationDetails.account,
      service: attestationDetails.service
    });
    attestationById[attestationCount] = attestationByHash[hashStr];

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

    emit AttestationCreated(
      profileId,
      attestationDetails.service,
      attestationDetails.account,
      evidence,
      attestationCount
    );
    attestationCount++;
  }

Repo Link

  function _claimAttestation(
    uint256 profileId,
    bytes32 attestationHash,
    string calldata evidence
  ) private returns (bool) {
    if (!attestationExistsForHash(attestationHash)) {
      return false;
    }

    Attestation memory attestation = attestationByHash[attestationHash];

    if (attestation.profileId == profileId) {
      return false;
    }

    address ethosProfile = _getEthosProfile();

    (bool profileExists, ) = ITargetStatus(ethosProfile).targetExistsAndAllowedForId(profileId);

    if (!profileExists) {
      revert ProfileNotFound(profileId);
    }
    bool senderBelongsToProfile = IEthosProfile(ethosProfile).addressBelongsToProfile(
      msg.sender,
      profileId
    );

    if (!senderBelongsToProfile) {
      revert AddressNotInProfile(msg.sender, profileId);
    }

    // Remove attestation from the previous profile
    _removeAttestationFromPreviousProfile(attestation.profileId, attestationHash);

    // Set new profileId for attestation
    attestationByHash[attestationHash].profileId = profileId;
    attestationHashesByProfileId[profileId].push(attestationHash);
    // Update the index of the hash in the new profile
    hashIndexByProfileIdAndHash[profileId][attestationHash] =
      attestationHashesByProfileId[profileId].length -
      1;

    // Restore attestation if it was previously archived
    if (attestationByHash[attestationHash].archived) {
      attestationByHash[attestationHash].archived = false;
    }

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

    emit AttestationClaimed(
      attestationByHash[attestationHash].attestationId,
      attestationByHash[attestationHash].service,
      attestationByHash[attestationHash].account,
      evidence,
      profileId
    );

    return true;
  }

Repo Link

Consider a profile's address has been hacked and then the profile user deletes the address from the profile and sets the isAddressCompromised[maliciousAddress] = true

But this address can still attest maliclous external accounts to this profile due to lack of check whether the address is compromised and destroy the reputation of the profile.

Impact

Compromised or hacked addresses can claim malicious external account attestations on behalf of the profile and ruin the reputation of the profile

Mitigation

Add a validation to check if the address is compromised in createAttestation():

if(IEthosProfile(ethosProfile).isAddressCompromised[msg.sender] )
    revert CompromisedAddress(msg.sender);