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

0 stars 0 forks source link

pkqs90 - `EthosProfile#registerAddress()` does not check for duplicate addresses. #202

Open sherlock-admin2 opened 2 weeks ago

sherlock-admin2 commented 2 weeks ago

pkqs90

Medium

EthosProfile#registerAddress() does not check for duplicate addresses.

Summary

EthosProfile#registerAddress does not check for duplicate addresses. Compromised addresses can abuse this to fill up address within a ProfileId.

Root Cause

For a single addressStr, the registerAddress() can be called to register the addressStr to a same profileId multiple times. Each time an address is registered, it is pushed inside the profiles[profileId].addresses array without checking for duplicates. If registered multiple times, there will be multiple instances of the address in the array.

This array is used to check the maximum amount of addresses within a profileId must not exceed maxNumberOfAddresses. So if a single address is pushed multiple times, that means there is less space for other addresses.

Note that using deleteAddressAtIndex to remove the address does not help, because removing an address would mean pushing the address in profiles[profileId].removedAddresses, which is also calculated when checking maximum amount of addresses.

There is a definition of "compromised user" in the EthosProfile, so if an address was compromised, he can request signatures from EthosSigner and register a single address multiple times to fill up the address array, so no new address can be added.

  function registerAddress(
    address addressStr,
    uint256 profileId,
    uint256 randValue,
    bytes calldata signature
  ) external whenNotPaused onlyNonZeroAddress(addressStr) {
    ...
@>  profiles[profileId].addresses.push(addressStr);

@>  checkMaxAddresses(profileId);

    emit AddressClaim(profileId, addressStr, AddressClaimStatus.Claimed);
  }

  /**
   * @dev Deletes an address at index.
   * @param addressIndex Index of address to be archived.
   */
  function deleteAddressAtIndex(uint256 addressIndex) external whenNotPaused {
    ...

    address[] storage removedAddresses = profiles[profileId].removedAddresses;

    ...

@>  _deleteAddressAtIndexFromArray(addressIndex, addresses, removedAddresses);

    emit AddressClaim(profileId, addressStr, AddressClaimStatus.Unclaimed);
  }

  function checkMaxAddresses(uint256 profileId) internal view {
    uint256 sum = profiles[profileId].addresses.length;
    sum += profiles[profileId].removedAddresses.length;
    if (sum > maxNumberOfAddresses) {
      revert MaxAddressesReached(profileId);
    }
  }

  function _deleteAddressAtIndexFromArray(
    uint256 index,
    address[] storage addresses,
    address[] storage removedAddresses
  ) private {
    address addr = addresses[addresses.length - 1];
    addresses[index] = addr;
    removedAddresses.push(addr);
    addresses.pop();
  }

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

Internal pre-conditions

  1. An address is compromised
  2. That address owner gains multiple signatures for registering an address to the same profileId.

External pre-conditions

N/A

Attack Path

N/A

Impact

Duplicate address will be registered for the same profileId, and new addresses can't be added.

PoC

Presented above.

Mitigation

Add a duplication check when registering addresses.