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

0 stars 0 forks source link

pashap9990 - last address always will be pushed to removedAddress instead of proper index #249

Open sherlock-admin4 opened 2 weeks ago

sherlock-admin4 commented 2 weeks ago

pashap9990

High

last address always will be pushed to removedAddress instead of proper index

Summary

last address always will be pushed to removedAddress instead of proper index

Root Cause

let's assume profile 1 has a,b,c as registered addresses and then he decide to remove address b, hence he should pass 1 as index to EthosProfile:deleteAddressAtIndex function which in turn call EthosProfile:_deleteAddressAtIndexFromArray with below parameters index => 1 addresses => [a, b, c] removedAddresses => []

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

as we can see addr which is last addresses will be added to removed address

Code Snippet

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

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

Impact

last address always will be pushed to removedAddress instead of proper index

PoC

Provided PoC ```typescript it("wrong-removed-address", async() => { const { ethosProfile, PROFILE_CREATOR_0, PROFILE_CREATOR_1, WRONG_ADDRESS_0, OWNER, EXPECTED_SIGNER, ADMIN } = await loadFixture(deployFixture); await ethosProfile.connect(OWNER).inviteAddress(PROFILE_CREATOR_0.address); await ethosProfile.connect(ADMIN).setDefaultNumberOfInvites(10); await ethosProfile.connect(PROFILE_CREATOR_0).createProfile(1); let profileCount = await ethosProfile.profileCount(); console.log("profile_count:", profileCount); let profile1 = await ethosProfile.getProfile(2); // await ethosProfile.connect(PROFILE_CREATOR_0).inviteAddress(PROFILE_CREATOR_1.address) console.log("profile1:", profile1); const profileId = String(await ethosProfile.profileIdByAddress(PROFILE_CREATOR_0.address)); const rand = '123'; let addressA = ethers.Wallet.createRandom().address; let addressB = ethers.Wallet.createRandom().address; let addressC = ethers.Wallet.createRandom().address; const signature = await common.signatureForRegisterAddress( addressA, profileId, rand, EXPECTED_SIGNER, ); const signature2 = await common.signatureForRegisterAddress( addressB, profileId, '1234', EXPECTED_SIGNER, ); const signature3 = await common.signatureForRegisterAddress( addressC, profileId, '12345', EXPECTED_SIGNER, ); await ethosProfile.connect(PROFILE_CREATOR_0).registerAddress(addressA, 2, rand, signature); await ethosProfile.connect(PROFILE_CREATOR_1).registerAddress(addressB, 2, '1234', signature2); await ethosProfile.connect(PROFILE_CREATOR_1).registerAddress(addressC, 2, '12345', signature3); console.log("proper address:", addressA); console.log("last address:", addressC); await ethosProfile.connect(PROFILE_CREATOR_0).deleteAddressAtIndex(1); profile1 = await ethosProfile.getProfile(2); console.log("profile1:", profile1); }); ```

Mitigation

@@ -587,8 +587,8 @@ contract EthosProfile is IEthosProfile, ITargetStatus, AccessControl, UUPSUpgrad
     address[] storage removedAddresses
   ) private {
     address addr = addresses[addresses.length - 1];
+    removedAddresses.push(addresses[index]);
     addresses[index] = addr;
-    removedAddresses.push(addr);
     addresses.pop();
   }