Albort - There’s an error in the _deleteAddressAtIndexFromArray function

[sherlock-admin4]

Albort

High

There’s an error in the _deleteAddressAtIndexFromArray function

Summary

https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosProfile.sol#L591 In the _deleteAddressAtIndexFromArray function, when an address is removed from the addresses array, the function should add the removed address to the removedAddresses array. However, the current implementation actually adds the last address to removedAddresses, which is incorrect.

Root Cause

In the deleteAddressAtIndex function, when an address is removed, it calls _deleteAddressAtIndexFromArray:

function deleteAddressAtIndex(uint256 addressIndex) external whenNotPaused {
    // ...some code omitted
    _deleteAddressAtIndexFromArray(addressIndex, addresses, removedAddresses);
    // ...some code omitted
}

However, in the _deleteAddressAtIndexFromArray function, the code is as follows:

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

Here, addr is set to addresses[addresses.length - 1], which is the last address in the array. This address is then added to the removedAddresses array. However, we should actually be adding the address at the specified index, the address being deleted.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

No response

Mitigation

Modify the _deleteAddressAtIndexFromArray function to correctly add the removed address to the removedAddresses array. The modified code should be as follows:

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

address addr = addresses[index];: First, retrieve the address to be deleted. removedAddresses.push(addr);: Add this address to the removedAddresses array. addresses[index] = addresses[addresses.length - 1];: Replace the deleted element with the last element in the array to maintain continuity. addresses.pop();: Remove the last element of the array.

With these modifications, the function will correctly handle address deletion, ensuring that the removed address is properly recorded, and the array’s state remains consistent.

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/trust-ethos/ethos/pull/1836