Lack of the limitation how many profileIds/followerProfileIds can be assigned into the `profileIds`/`followerProfileIds` array parameter at once, the transaction will be reverted due to reaching the gas limit in the while-loop

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/misc/LensV2Migration.sol#L34 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/misc/LensV2Migration.sol#L42 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L38 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L43-L44 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L95 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L106-L107

Vulnerability details

Impact

Within the MigrationLib#batchMigrateProfiles(), there is no limitation how many profileIds can be assigned into the profileIds array parameter at once. If a caller assign too many profileIds into the the profileIds array parameter at once, the transaction will be reverted due to reaching the gas limit in the while-loop in the MigrationLib#batchMigrateProfiles().

Also, within the MigrationLib#batchMigrateFollows(), there is no limitation how many followerProfileIds can be assigned into the followerProfileIds array parameter at once. If a caller assign too many followerProfileIds into the the followerProfileIds array parameter at once, the transaction will be reverted due to reaching the gas limit in the while-loop in the MigrationLib#batchMigrateFollows().

Proof of Concept

Within the LensV2Migration, the MigrationLib#batchMigrateProfiles() would be called like this: https://github.com/code-423n4/2023-07-lens/blob/main/contracts/misc/LensV2Migration.sol#L34

    function batchMigrateProfiles(uint256[] calldata profileIds) external {
        MigrationLib.batchMigrateProfiles(profileIds, lensHandles, tokenHandleRegistry); /// @audit
    }

Also, within the LensV2Migration, the MigrationLib#batchMigrateFollows() would be called like this: https://github.com/code-423n4/2023-07-lens/blob/main/contracts/misc/LensV2Migration.sol#L42

    function batchMigrateFollows(
        uint256[] calldata followerProfileIds,
        uint256[] calldata idsOfProfileFollowed,
        uint256[] calldata followTokenIds
    ) external {
        MigrationLib.batchMigrateFollows(followerProfileIds, idsOfProfileFollowed, followTokenIds); /// @audit
    }

Within the MigrationLib#batchMigrateProfiles(), the profileIds would be assigned into the profileIds array parameter. And then, the MigrationLib#_migrateProfile() would be called multiple times based on the number of profileIds stored in the profileIds array parameter like this: https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L38 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L43-L44

    /**
     * @notice Migrates an array of profiles from V1 to V2. This function can be callable by anyone.
     * We would still perform the migration in batches by ourselves, but good to allow users to migrate on their own if they want to.
     *
     * @param profileIds The array of profile IDs to migrate.
     */
    function batchMigrateProfiles(
        uint256[] calldata profileIds,  /// @audit
        LensHandles lensHandles,
        TokenHandleRegistry tokenHandleRegistry
    ) external {
        uint256 i;
        while (i < profileIds.length) { /// @audit
            _migrateProfile(profileIds[i], lensHandles, tokenHandleRegistry);   /// @audit
            unchecked {
                ++i;
            }
        }
    }

Within the MigrationLib#batchMigrateFollows(), the followerProfileIds would be assigned into the followerProfileIds array parameter. And then, the MigrationLib#_migrateFollow() would be called multiple times based on the number of followerProfileIds stored in the followerProfileIds array parameter like this: https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L95 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/MigrationLib.sol#L106-L107

    // FollowNFT Migration:

    function batchMigrateFollows(
        uint256[] calldata followerProfileIds,    /// @audit
        uint256[] calldata idsOfProfileFollowed,
        uint256[] calldata followTokenIds
    ) external {
        if (
            followerProfileIds.length != idsOfProfileFollowed.length ||
            followerProfileIds.length != followTokenIds.length
        ) {
            revert Errors.ArrayMismatch();
        }
        uint256 i;
        while (i < followerProfileIds.length) { /// @audit
            _migrateFollow(followerProfileIds[i], idsOfProfileFollowed[i], followTokenIds[i]); /// @audit
            unchecked {
                ++i;
            }
        }
    }

However, within the MigrationLib#batchMigrateProfiles(), there is no limitation how many profileIds can be assigned into the profileIds array parameter at once. If a caller assign too many profileIds into the the profileIds array parameter at once, the transaction will be reverted due to reaching the gas limit in the while-loop in the MigrationLib#batchMigrateProfiles().

Also, within the MigrationLib#batchMigrateFollows(), there is no limitation how many followerProfileIds can be assigned into the followerProfileIds array parameter at once. If a caller assign too many followerProfileIds into the the followerProfileIds array parameter at once, the transaction will be reverted due to reaching the gas limit in the while-loop in the MigrationLib#batchMigrateFollows().

Tools Used

• Foundry

Recommended Mitigation Steps

Within the MigrationLib#batchMigrateProfiles(), consider implementing a setter function to set a limitation how many profileIds can be assigned into the profileIds array parameter at once. In addition to that, consider adding a input validation whether or not the profileIds assigned into the the profileIds array parameter would exceed the limitation like this:

+    uint256 MAX_NUMBER_OF_PROFILE_IDS_AT_ONCE;
...

+    function setMaxNumberOfProfileIdsAtOnce(uint256 _MaxNumberOfProfileIdsAtOnce) public onlyOwner {
+.       MAX_NUMBER_OF_PROFILE_IDS_AT_ONCE = _MaxNumberOfProfileIdsAtOnce;
+    }

    ...
    function batchMigrateProfiles(
        uint256[] calldata profileIds, 
        LensHandles lensHandles,
        TokenHandleRegistry tokenHandleRegistry
    ) external {
+       require(profileIds.length < MAX_NUMBER_OF_PROFILE_IDS_AT_ONCE, "profileIds.length must be less than the MAX_NUMBER_OF_PROFILE_IDS_AT_ONCE");

        uint256 i;
        while (i < profileIds.length) {
            _migrateProfile(profileIds[i], lensHandles, tokenHandleRegistry); 
            unchecked {
                ++i;
            }
        }
    }

Also, within the MigrationLib#batchMigrateFollows(), consider setting a limitation how many profileIds can be assigned into the followerProfileIds array parameter at once. In addition to that, consider adding a input validation whether or not the profileIds assigned into the the followerProfileIds array parameter would exceed the limitation like this:

+    uint256 MAX_NUMBER_OF_FOLLOWER_PROFILE_IDS_AT_ONCE;
...

+    function setMaxNumberOfFollowerProfileIdsAtOnce(uint256 _MaxNumberOfFollowerProfileIdsAtOnce) public onlyOwner {
+.       MAX_NUMBER_OF_FOLLOWER_PROFILE_IDS_AT_ONCE = _MaxNumberOfFollowerProfileIdsAtOnce;
+    }

    ...
    function batchMigrateFollows(
        uint256[] calldata followerProfileIds,
        uint256[] calldata idsOfProfileFollowed,
        uint256[] calldata followTokenIds
    ) external {
+       require(followerProfileIds.length < MAX_NUMBER_OF_FOLLOWER_PROFILE_IDS_AT_ONCE, "followerProfileIds.length must be less than the MAX_NUMBER_OF_FOLLOWER_PROFILE_IDS_AT_ONCE");    

        if (
            followerProfileIds.length != idsOfProfileFollowed.length ||
            followerProfileIds.length != followTokenIds.length
        ) {
            revert Errors.ArrayMismatch();
        }
        uint256 i;
        while (i < followerProfileIds.length) {
            _migrateFollow(followerProfileIds[i], idsOfProfileFollowed[i], followTokenIds[i]);
            unchecked {
                ++i;
            }
        }
    }

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #11

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid