`_ensureAvatarsRegistered`is not correctly returning the `registrationCount`

Github username: -- Twitter username: -- Submission hash (on-chain): 0x7a9e1a72521a2962d7f43acd2baed5d5ab6b214c1d0330333a25d816b5aa605c Severity: medium

Description: Description

As per migration rule, v1 circle owners can apply for migration within the bootsrap window where they no need to pay the INVITATION_COST cost. Once they pass the invitationOnlyTime, they have to pay the invitation cost which is set by the owner.

This cost apply is not correctly enforced when the migartion happen through the migration contract.

As per current implementation, the migrate function allows to be called by anyone with valid human address. it does not check whether the avatar is already migrated.

The circle are transferred from the call to the migration contract and then the migrate function is called in the Hub.sol.

In the Hub.sol, once the invitation period is passed, the invitation cost is applied and then the tokens are minted

    function migrate(address _owner, address[] calldata _avatars, uint256[] calldata _amounts) external onlyMigration {
       ---some code---
        // register all unregistered avatars as humans, and check that registered avatars are humans
        // after the bootstrap period, the _owner needs to pay the equivalent invitation cost for all newly registered humans
        uint256 cost = INVITATION_COST * _ensureAvatarsRegistered(_avatars); ---<< audit find

        // Invitation cost only applies after the bootstrap period
        if (block.timestamp > invitationOnlyTime && cost > 0) {
            // personal Circles are required to burn the invitation cost
            if (!isHuman(_owner)) {
                // Only humans can migrate v1 tokens after the bootstrap period.
                revert CirclesHubMustBeHuman(_owner, 4);
            }
            _burnAndUpdateTotalSupply(_owner, toTokenId(_owner), cost);
        }

        for (uint256 i = 0; i < _avatars.length; i++) {
            // mint the migrated balances to _owner
            _mintAndUpdateTotalSupply(_owner, toTokenId(_avatars[i]), _amounts[i], "");
        }
    }

lets look at the function _ensureAvatarsRegistered.

    function _ensureAvatarsRegistered(address[] calldata _avatars) internal returns (uint256) {
        uint256 registrationCount = 0;
        for (uint256 i = 0; i < _avatars.length; i++) {
            if (avatars[_avatars[i]] == address(0)) {
                registrationCount++;
                _registerHuman(_avatars[i]);
            } else {
                if (!isHuman(_avatars[i])) {
                    // Only humans can be registered.
                    revert CirclesHubMustBeHuman(_avatars[i], 5);
                }
            }
        }

        return registrationCount;
    }

if the avatar is already registered, it checks if it is valid human. When the avatar already exists in the avatars map, the revert will happen inside _registerHuman function call.

Note, only avatars[_avatars[i]] == address(0), the registrationCount will be updated.

Impact

lesser invitation cost is burned from the caller (from the migration.sol). Other transferred token left inside the migration.sol
invariat check is incorrectly done - if avatars[_avatars[i]] already exists, the function _ensureAvatarsRegistered is not reverting.

Revised Code File (Optional)

Inside the _ensureAvatarsRegistered, revert if avatars[_avatars[i]] != address(0)

    function _ensureAvatarsRegistered(address[] calldata _avatars) internal returns (uint256) {
        uint256 registrationCount = 0;
        for (uint256 i = 0; i < _avatars.length; i++) {
            if (avatars[_avatars[i]] == address(0)) {
                registrationCount++;
                _registerHuman(_avatars[i]);
            } else {
---               if (!isHuman(_avatars[i])) {
---                // Only humans can be registered.
                    revert CirclesHubMustBeHuman(_avatars[i], 5); -->> change the revert case.
                }
            }
        }

        return registrationCount;
    }

hats-finance / Circles-0x6ca9ca24d78af44582951825bef9eadcb210e5cf

`_ensureAvatarsRegistered`is not correctly returning the `registrationCount` #70