Skip to content

Latest commit

 

History

History
176 lines (135 loc) · 7.75 KB

032.md

File metadata and controls

176 lines (135 loc) · 7.75 KB
Raw

Quiet Ocean Boar

Medium

invariant One may not write to Ethos contracts until a (non-mock) profile is created. is broken

Summary

The protocol describes some invariants that should not be broken, these invariants can be observed below:

One address may be associated with maximum one profile. Profile creation requires at least one active invitation. One may not write to Ethos contracts until a (non-mock) profile is created.

to be specific the invariant One may not write to Ethos contracts until a (non-mock) profile is created. is broken because it is possible for a user to call archiveProfile , restoreProfile, and registerAddress with a mock account, and thus breaking the invariant since the functions archiveProfile , restoreProfile, and registerAddress writes to an ethos contract.

Root Cause

in EthosReview.sol ln 174

https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosReview.sol#L173

  function addReview(
    Score score,
    address subject,
    address paymentToken,
    string calldata comment,
    string calldata metadata,
    AttestationDetails calldata attestationDetails
  ) external payable whenNotPaused {

the function addReview allows a profile to leave a review for a subject. If the subject does not hold an ethos profile, a mock will be made for them. The problem is that by inputting specific values, a user can make a mock account and then call archiveProfile , restoreProfile, and registerAddress to write to the the ethosProfile contract and thus breaking the invariant.

Let me breakdown how this is done...

first a user must call addReview with subject that != 0 and a subject address that does not currently own a profile on the EthosProfile contract. The subject Address must be owned by the user to pull the remaining part of the exploit of aswell.

now lets go line by line to see the outcome...

_validateReviewDetails(subject, attestationDetails);

with the following configuration this check will pass next...

    if (subject != address(0)) {
      mockId = ethosProfile.profileIdByAddress(subject);
      mockId = _addReview(mockId, subject, false, 0x0, ethosProfile);

This if statement will be triggered which is what we need to pull of the exploit, the call ethosProfile.profileIdByAddress(subject) will return 0 because as i stated before subject address is currently not associated with a profileId and thus the default value will be returned which is 0, next mockID is set to 0 and we call _addReview with the mockID = 0. Lets observe the consequence of this

  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) {
      subjectProfileId = ethosProfile.incrementProfileCount(
        isAttestation,
        subject,
        attestationHash
      );

the first if statement is triggered and thus we will call incrementProfileCount let us see what happens next

  function incrementProfileCount(
    bool isAttestation,
    address subject,
    bytes32 attestation
  ) external whenNotPaused returns (uint256 profileId) {
    if (
      msg.sender != contractAddressManager.getContractAddressForName(ETHOS_REVIEW) &&
      msg.sender != contractAddressManager.getContractAddressForName(ETHOS_ATTESTATION)
    ) {
      revert InvalidSender();
    }
    profileId = profileCount;
    if (isAttestation) {
      profileIdByAttestation[attestation] = profileId;
    } else {
      profileIdByAddress[subject] = profileId;
    }
    profileCount++;

    emit MockProfileCreated(profileId);
  }

We hit the else statement and we assign the subjectAddress (which is the users other wallet) to the next profileID

Now that the users wallet has been assigned a profileID the user has officially associated his address with a mock account. And since the mock account has a profileId, it no longer returns variable mock as true in the following functions:

  function profileStatusById(
    uint256 profileId
  ) public view returns (bool verified, bool archived, bool mock) {
    Profile storage profile = profiles[profileId];
    verified = profile.profileId > 0;
    archived = verified && profile.archived;
    mock = profileId > 0 && !verified && profileId < profileCount;
  }

  /**
   * @dev Returns the status of a profile by its associated address.
   * @param addressStr The address to check.
   * @return verified Whether the profile is verified.
   * @return archived Whether the profile is archived.
   * @return mock Whether the profile is a mock profile.
   * @return profileId The ID of the profile associated with the address.
   */
  function profileStatusByAddress(
    address addressStr
  ) public view returns (bool verified, bool archived, bool mock, uint256 profileId) {
    profileId = profileIdByAddress[addressStr];
    (verified, archived, mock) = profileStatusById(profileId);
  }

these 2 checks are important to ensure a mock cannot write to the ethos contracts and i have shown how a user can take control of a mock account and with the same mock account bypass these checks. Now that he can pass these checks the mock account can do almost everything a regular account can do. Including calling archiveProfile , restoreProfile, and registerAddress. (note: can also call function addReview, addReply, and voteFor found this out after writing report)

Therefore the invariant One may not write to Ethos contracts until a (non-mock) profile is created. is broken.

and according to sherlock docs;

The protocol team can use the README (and only the README) to define language that indicates the codebase's restrictions and/or expected functionality. Additionally, the protocol team can use only the following question to define the protocol's invariants/properties:

What properties/invariants do you want to hold even if breaking them has a low/unknown impact?

Issues that break the invariants from the above question, irrespective of whether the impact is low/unknown, will be assigned Medium severity. High severity will be applied only if the issue falls into the High severity category in the judging guidelines.

Example: The README states "Admin can only call XYZ function once" but the code allows the Admin to call XYZ function twice; this is a valid Medium

This will qualify this issue as a valid medium.

Internal pre-conditions

  1. address must not have a profile on ethosprofile

External pre-conditions

none

Attack Path

  1. the address does not have an ethos profile associated to it
  2. the user must call addReview with the address
  3. the address will be assigned to a mock
  4. the address can call archiveProfile , restoreProfile, and registerAddress
  5. a mock has been used to write to an ethos contract
  6. the invariant One may not write to Ethos contracts until a (non-mock) profile is created. is broken

Impact

breaking of a protocol invariant, allows a mock to write to an ethos contract

PoC

No response

Mitigation

Since the mock the user is controlling does not set the following values

    profiles[profileId].profileId = profileId;
    profiles[profileId].createdAt = block.timestamp;
    profiles[profileId].inviteInfo.available = defaultNumberOfInvites;
    profiles[profileId].addresses.push(user);

We can prevent a mock account controlled by a user to interact with the protocol by using a check createdAt>0 in the functions archiveProfile , restoreProfile, and registerAddress

This fix is not perfect so id recommend thinking about a fix that is more in depth and ensures that the invariant holds.