Users can avoid blocking by making calling _followWithWrappedToken themselves or make an approval to another followerProfileId

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L196 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L426 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L77

Vulnerability details

Impact

In FollowNFT.sol, there is a processBlock() function that the owner of the FollowNFT collection can use to block his followers. However, before deleting all the state related to the followTokenId, the new ERC721 token is minted for the same tokenId (wrapped) so that the user is not following anymore but at the same time doesn't lose the token. The owner of this tokenId can make approvals and let other users follow the owner of the Follow NFT collection. This happens because when calling processBlock(), except for minting new wrapped token, the internal _unfollow() is called. Inside of _unfollow(), all the state related to this particular followTokenId is deleted (returned to the default values) but there is nothing to prevent an approved user to follow again.

Proof of Concept

Imagine the following scenario:

1. Alice has followTokenId 5 of the Bob's FollowNFT collection. There is special content that is created for the first 10 followers so, in addition to holding the token that Alice can sell, she gets some premium content.

2. Bob decides to block Alice by calling processBlock() and she basically unfollows but saves the token as it's minted before calling internal _unfollow():

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

3. All the mappings are returned back to the default values but Alice has wrapped token:

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

4. Alice approves another user and he calls follow() with the same tokenId - 5. His profileId is not blocked so the checks in ValidationLib are bypassed. Then, as the unsafeOwnerOf() != address(0), the following will still be possible and _followWithWrappedToken() will be called:

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

5. _followWithWrappedToken() doesn't check whether the followTokenId was blocked and it just calls _replaceFollower(). _replaceFollower() checks whether the currentProfileId != 0 and if it's (like in this case), it just increment _followerCount and calls _baseFollow. This allows the approved person to follow with the same tokenId the owner's Follow NFT collection again while being blocked before and to get the premium content from Bob.

As Lens allows to create more than 1 profileIds for the same person, Alice can just give an approval to herself (another profileId), it doesn't necessarily has to be another person and the block will be bypassed.

Tools Used

Manual review.

Recommended Mitigation Steps

Add some checks to prevent approved users from being able to follow with the same tokenId if they were blocked.

Assessed type

Other

[c4-sponsor]

donosonaumczuk marked the issue as sponsor disputed

[donosonaumczuk]

• The fact that the same Follow token can be used to follow again is by design, so even when the user is blocked, it can still sell the NFT or rent it.
• Take into account that it is always up to the Follow module to allow this, as a follow module can only allow "fresh follows" if desired.
• A profile that is blocked can never follow again, this check is not in the Follow NFT but in the LensHub. See FollowLib::follow, a call to ValidationLib::validateNotBlocked.
• The fact that a blocked user can create a new account and follow again is accepted. The same happens in every social platform. Apps built on Lens can perform extra checks if they want to take into account IPs, KYC or any other thing, but for the protocol itself this is ok.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[rodiontr]

Hi @Picodes ! Thank you for your judging!

This submission was slightly edited and I forgot to change the name of the submission - the owner can't follow with his wrapped token as his profileId is blocked - sorry for that! However, It can be seen from the explanation that the issue is in approve() functionality where the user can just approve() to another profileId while being blocked. This issue emphasizes on how user can avoid blocking just by using approve() function and he basically reserves his tokenId number. Sponsor’s point of view is that it’s expected behavior but I think the main problem here is that this allows the owner to reserve the tokenId that possibly give him some privileges while being blocked. I agree that in every social media the user can just create the new account but here the situation is slightly different as in social media there are no tokenIds. So for the user to be able to just approve the same tokenId to his another profileId just indicates that blocking functionality doesn't work. And this tokenId may give some priviliges (see comment on issue #93).

I think it should be like this:

1) the user is blocked - he can create another profileId and follow again with another tokenId - this is expected behavior 2) the user is blocked - he can create another profileId and follow with the same reserved tokenId while being blocked by giving newly created profileId an approval - unexpected behavior (as in the current implementation) - in other words, what's the point of being blocked here if it can be so easily bypassed ? And another question - in regular social media, ids have no any value while in this protocol they can be widely used for "renting", for example (issue #93)

Thank you for reviewing !

[Picodes]

@rodiontr I side with the sponsor's side on this one. To me the difference between "creating another profileId and follow again with another tokenId" and "creating another profileId and follow again with the same tokenId" is tight, but makes sense as blocking is per profile and not per token