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

0 stars 0 forks source link

LeFy - Reregistering an already deleted address is not implemented correctly #223

Open sherlock-admin2 opened 3 weeks ago

sherlock-admin2 commented 3 weeks ago

LeFy

High

Reregistering an already deleted address is not implemented correctly

Summary

In 'EthosProfile.sol', a user can delete a registered address and can also reregister the same address again, but reregistering the address is not handled properly as the address is not deleted from the profiles[profileId].removedAddresses and also its compromised status is also not updated.

Root Cause

When a user deletes an address from their profile, its added to the profiles[profileId].removedAddresses list:

  function deleteAddressAtIndex(uint256 addressIndex) external whenNotPaused {
    uint256 profileId = profileIdByAddress[msg.sender];
    (bool verified, bool archived, bool mock) = profileStatusById(profileId);
    if (!verified) {
      revert ProfileNotFoundForAddress(msg.sender);
    }
    if (archived || mock) {
      revert ProfileAccess(profileId, "Profile is archived");
    }

    address[] storage addresses = profiles[profileId].addresses;
    address[] storage removedAddresses = profiles[profileId].removedAddresses;
    if (addresses.length <= addressIndex) {
      revert InvalidIndex();
    }

    address addressStr = addresses[addressIndex];
    isAddressCompromised[addressStr] = true;
    _addressShouldDifferFromSender(addressStr);

    _deleteAddressAtIndexFromArray(addressIndex, addresses, removedAddresses);

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

Repo Link

The user can reregister once deleted address by calling the registerAddress():

function registerAddress(
    address addressStr,
    uint256 profileId,
    uint256 randValue,
    bytes calldata signature
  ) external whenNotPaused onlyNonZeroAddress(addressStr) {
    (bool verified, bool archived, bool mock) = profileStatusById(profileId);
    if (!verified) {
      revert ProfileNotFound(profileId);
    }
    if (archived || mock) {
      revert ProfileAccess(profileId, "Profile is archived");
    }
    // you may restore your own previously deleted address,
    // but you cannot register an address that has been deleted by another user
    if (profileIdByAddress[addressStr] != profileId && isAddressCompromised[addressStr]) {
      revert AddressCompromised(addressStr);
    }
    (bool addressAlreadyRegistered, , , uint256 registeredProfileId) = profileStatusByAddress(
      addressStr
    );
    if (addressAlreadyRegistered && registeredProfileId != profileId) {
      revert ProfileExistsForAddress(addressStr);
    }

    validateAndSaveSignature(
      _keccakForRegisterAddress(addressStr, profileId, randValue),
      signature
    );

    profiles[profileId].addresses.push(addressStr);
    profileIdByAddress[addressStr] = profileId;

    checkMaxAddresses(profileId);

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

Repo Link

But when the user reregisters the address , its not removed from the removedAddresses list and its compromised status is also not set to false and the same address will have active and deleted status at the same time and cause issues

Impact

On reregistering an address to a profile it will have both active and deleted/compromised status.

Mitigation

In registerAddress() function add a check to see if its a reregistering scenario(either check if the address is present in profiles[profileId].removedAddresses or if the address is compromised) and if its true then remove the address from the removedAddresses list and set the isAddressCompromised[addressStr] to false: