Lens Protocol V2 - Emmanuel's results

Lines of code

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

Vulnerability details

Impact

A user can follow multiple times, specifically by first following through LensHub#follow, which then calls FollowNFT#follow. After that, the user can call LensV2Migration#batchMigrateFollows if they had a pre-upgrade followNFT. Furthermore, they can repeat this process as many times as they have pre-upgrade NFTs accessible to them.

Proof of Concept

The FollowNFT#tryMigrate function does not verify if the profileId being migrated is already following.

A malicious user with a pre-upgrade FollowNFT can exploit this by first calling FollowNFT#follow through LensHub and then proceeding to migrate their FollowNFT.

As a result, this leads to double following, and in a scenario where everyone follows this pattern, a profile will end up with twice as many followers as it should have. However, these followers will only be backed by half the number of unique profiles due to the duplication.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure that the followerProfileId in FollowNFT#tryMigrate is not following already. A similar check used in follow function can be used here:

if (_followTokenIdByFollowerProfileId[followerProfileId] != 0) {
    revert AlreadyFollowing();
}

Assessed type

Invalid Validation

Judge has assessed an item in Issue #112 as 2 risk. The relevant finding follows:

A profile can follow itself by receiving a pre-upgrade followNFT and then using the batchMigrateFollows function."

Lines of code

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

Vulnerability details

Impact

Adversary that possesses a followNFT that was minted before the upgrade, can send it to a target address that owns a particular profile and then execute the LensV2Migration#batchMigrateFollows function. By providing the necessary inputs such as the profileId, idOfProfileFollowed, and the transferred tokenId, the adversary successfully enforces the target profile to follow the specified profile.

This introduces several potential attacks:

• An adversary can force any profile they wish to follow a particular profile.
• The adversary can forcefully change the tokenId that an already migrated user or post-upgrade minter is registered with.
• If a profile gets blocked, the user can refollow by accepting a pre-upgrade followNFT and then using the batchMigrateFollows function.
• A profile can follow itself by receiving a pre-upgrade followNFT and then using the batchMigrateFollows function."

Proof of Concept

The tryMigrate function does not consider the fact that any address will be forced to be the ownerOf a tokenId if it gets sent that token. This is common to all NFTs. Alice cannot stop Bob from sending her his NFTs.

For follow tokens that were minted before the upgrade, the batchMigrateFollows which downstream calls the tryMigrate function, is used to re-register the profileId that owns those tokens and grant them functionalities equivalent to those of tokens minted after the upgrade.

As an owner of a pre-upgrade followNFT, you can forcefully register ANY address with your tokenId. This can be achieved by simply sending your followNFT to an address that owns a specific profileId and then calling the LensV2Migration#batchMigrateFollows function, providing that profileId, idOfProfileFollowed, and the tokenId you just transferred.

Here are the checks that tryMigrate does:

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

If the tokenId has not been used in migration before, and the adversary's input parameters are valid, and the tokenId exists, the first three checks will pass. The last check ensures that the owner of the followTokenId is the same as the followerProfileOwner. Since the adversary can control the ownerOf a followTokenId (by sending it to any address they wish) and can also provide any profile as an input parameter, this check will pass. Consequently, it would lead to the registration of that profile as a follower of any profile the adversary desires.

Tools Used

Manual Review

Recommended Mitigation Steps

I suggest allowing only the protocol or current owner of a followNFT to migrate his followNFT. Normal users should not be able to migrate another profile's followNFT unless with a signature. This way, if a malicious user sends his followNFT to another address, he won't be able to tryMigrate because he is not the current owner of the token, while the token recipient can tryMigrate at will.

Assessed type

Invalid Validation

Judge has assessed an item in Issue #112 as 2 risk. The relevant finding follows:

If a profile gets blocked, the user can refollow by accepting a pre-upgrade followNFT and then using the batchMigrateFollows function.

Low 001

Title

First follower is spending more gas

Impact

the first follower of a profile is spending more gas than subsequent followers. This will be unfair to them.

Proof of Concept

As we can see in FollowLib#_follow, if that profile has no followNFT, the first follower does the deployment, which will consume more gas.

function _follow()private returns (uint256){
    ...
    address followNFT = _profileToFollow.followNFT;
    if (followNFT == address(0)) {
        followNFT = _deployFollowNFT(idOfProfileToFollow);
        _profileToFollow.followNFT = followNFT;
    }
    ...
}

Tools Used

Manual Review

Recommendation

Consider deploying a followNFT at profile creation, so profile owner pays for that, while all followers spend same amount of gas to follow.

Low 002

Title

Non EOA is allowed to have a profile and follow other profiles

Impact

I think this is an issue because a user that wants a lot of followers can have many contract accounts follow him. Also, this would lead to bad UX in frontend as there will be a lot of fake profiles.

Given the peculiarity of this protocol, I consider this to be a valid issue.

Proof of Concept

Looking at LensHub#createProfile function, we can see that there is no check that prevents a non EOA from being the createProfileParams.to. This will allow minting of profiles to a non EOA account.

Tools Used

Manual Review

Recommendation

Consider adding an onlyEOA modifier to LensHub#createProfile