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

0 stars 0 forks source link

Bozho - Compromised addresses can interact with the whole system and grief other users #282

Open sherlock-admin4 opened 2 weeks ago

sherlock-admin4 commented 2 weeks ago

Bozho

High

Compromised addresses can interact with the whole system and grief other users

Summary

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

If a user removes an address from their profile, the address becomes marked as compromised and is intended to be restricted with the system. This is confirmed in the documentation and by the sponsor:

  • isAddressCompromised: (mapping(address => bool)) Keeps track of addresses that are compromised, preventing certain actions from being taken by these addresses.

documentation/ethosProfile.md

Private thread with the sponsor: sponsor confirming

However, the archiveAttestation() and restoreAttestation() functions in EthosAttestation.sol do not check if the address is compromised, allowing it to perform these actions.

334:  function archiveAttestation(bytes32 attestationHash) external whenNotPaused {
        ...
        // ensure attestation belongs to sender
        uint256 profileId = attestationByHash[attestationHash].profileId;
  📌    bool senderBelongsToProfile = IEthosProfile(_getEthosProfile()).addressBelongsToProfile(msg.sender, profileId);
        if (!senderBelongsToProfile) {
            revert AddressNotInProfile(msg.sender, profileId);
        }
      ...
    }

EthosAttestation.sol#342

532: function addressBelongsToProfile(address addressStr, uint256 profileId) external view returns (bool) {
        ...
        return profileIdByAddress[addressStr] == profileId;
    }

EthosProfile.sol#539

The function addressBelongsToProfile() only checks if the address is mapped to the profile ID in the profileIdByAddress mapping but does not consider whether the address is compromised. As a result, compromised addresses can still interact with the system, which violates the intended design.

Due to verifiedProfileIdForAddress() and profileStatusByAddress() also being vulnerable by this issue, the following functions can be also used by a compromised address:

Root Cause

checkIsAddressCompromised() checks are missing inside profileStatusByAddress(), verifiedProfileIdForAddress(), and addressBelongsToProfile() functions.

Internal pre-conditions

User has a compromised address.

External pre-conditions

None

Attack Path

  1. UserA's address becomes compromised.
  2. The malicious actor with access to the compromised address can interact with the system as if it is not compromised.
  3. Malicious actor leaves neagtive reviews, votes, comments, etc.

Impact

Compromised addresses can interact with the system as if they were not compromised, allowing malicious actions (e.g., leaving negative reviews, votes, comments, etc), which can impact users' social status. The original profile owner is unable to remove the compromised address, meaning damage done by the compromised address cannot be undone.

The profile owner needs to create a new profile to get rid of the compromised address.

PoC

The following PoC demonstrates how a compromised address can archive and restore attestations. But the same concept can be applied to other functions mentioned above.

Place the code in attestation.create.test.ts:

Click to see the code ```typescript // eslint-disable-next-line jest/no-focused-tests it.only('should be able to archive and restore attestation from compromised profile', async () => { const signature = await common.signatureForCreateAttestation( '2', '3592832', ACCOUNT_NAME_BEN, SERVICE_X, ATTESTATION_EVIDENCE_0, EXPECTED_SIGNER, ); await ethosAttestation .connect(userA.signer) .createAttestation( 2, 3592832, { account: ACCOUNT_NAME_BEN, service: SERVICE_X }, ATTESTATION_EVIDENCE_0, signature, ); const aHash = await ethosAttestation.getServiceAndAccountHash(SERVICE_X, ACCOUNT_NAME_BEN); const attestation = await ethosAttestation.getAttestationByHash(aHash); expect(attestation.archived).to.be.equal(false); const blocked = await deployer.newWallet(); const signatureForRegistration = await common.signatureForRegisterAddress( blocked.address, (2).toString(), (222323).toString(), EXPECTED_SIGNER, ); await ethosProfile .connect(userA.signer) .registerAddress(blocked.address, 2, 222323, signatureForRegistration); await ethosProfile.connect(userA.signer).deleteAddressAtIndex(1); await expect(ethosProfile.checkIsAddressCompromised(blocked.address)) .to.be.revertedWithCustomError(ethosProfile, 'AddressCompromised') .withArgs(ethers.getAddress(blocked.address)); // should try to archive attestation from compromised profile await ethosAttestation.connect(blocked).archiveAttestation(aHash); const attestationAfterArchive = await ethosAttestation.getAttestationByHash(aHash); expect(attestationAfterArchive.archived).to.be.equal(true); // should try to restore attestation from compromised profile await ethosAttestation.connect(blocked).restoreAttestation(aHash); const attestationAfterRestore = await ethosAttestation.getAttestationByHash(aHash); expect(attestationAfterRestore.archived).to.be.equal(false); }); ```

Mitigation

Add checkIsAddressCompromised()@EthosProfile to addressBelongsToProfile() and profileStatusByAddress() functions.