y4y - Attestation can be forced restored out of archive in `EthosAttestation::_claimAttestation`

[sherlock-admin3]

y4y

Medium

Attestation can be forced restored out of archive in EthosAttestation::_claimAttestation

Summary

When creating an attestation, users can take an existing attestation by providing sufficient evidence and get valid signature off-chain. During creation, there are several checks done, one of them is restoring archived attestation if can. However, due to a faulty logic, some special attestation can be forced restored out of archive.

Root Cause

Here is part of the createAttestation function:

  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;
    }
    // ... snip

We see, after _claimAttestation, restoreIfArchived is called to get attestation out of archive. Eventually, restoreAttestation is called:

  function restoreAttestation(bytes32 attestationHash) public whenNotPaused {
    uint256 profileId = attestationByHash[attestationHash].profileId;

    address ethosProfile = _getEthosProfile();

    (, bool isArchived) = IEthosProfile(ethosProfile).profileExistsAndArchivedForId(profileId);

    if (isArchived) {
      revert ProfileNotFound(profileId);
    }

    bool senderBelongsToProfile = IEthosProfile(ethosProfile).addressBelongsToProfile(
      msg.sender,
      profileId
    );

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

    if (!attestationByHash[attestationHash].archived) {
      revert AttestationNotArchived(attestationHash);
    }

    attestationByHash[attestationHash].archived = false;

    emit AttestationRestored(
      attestationByHash[attestationHash].attestationId,
      attestationByHash[attestationHash].service,
      attestationByHash[attestationHash].account,
      profileId
    );
  }

One of the check is if the profile is archived or not, and an archived profile's attestation will not be restored based on the logic in function. Now, let's take a look at _claimAttestation:

  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; // <=(1)
    }

    // 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;
  }

In (1), attestationByHash[attestationHash].archived is set to false if previously set to true. Right after this function, restoreIfArchived is called, and this would get an archived profile's attestation restored regardless. The inconsistency here would cause unexpected and unauthorized restoration of attestation, despite sufficient evidences are provided to create such attestation.

Internal pre-conditions

There are two profiles, A and B. Profile A has associated attestation D, but profile A and its attestation D are both archived. Normally, in order to restore attestation D, profile A has to be not in archived state.

External pre-conditions

No response

Attack Path

No response

Impact

This breaks one of the logic, which archived profile's archived attestation cannot be restored.

PoC

No response

Mitigation

Add another archived profile check in _claimAttestation as well.