Business Logic Flaw in Follow/Unfollow Functionality in LensV2Follow Contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/4ad521f84c43771d365ec0b35f904158201943fb/contracts/LensHub.sol#L368

Vulnerability details

Impact

Looking closely at the LensV2Follow contract, I think I've stumbled upon something interesting. It seems like there's a logic flaw in how the follow/unfollow system is implemented which could potentially allow some crafty users to artificially increase follower counts. Let's dig in!

When a user (say, User A) opts to unfollow another user (User B), the token associated with User A's 'follow' action transitions into an "unwrapped" state. Simultaneously, User A is granted the ability to recover this token. This is effectively handled by the unfollow function:

 if (followTokenOwner == address(0)) {
        // Follow token is unwrapped.
        // Unfollowing and allowing recovery.
        _unfollow({unfollower: unfollowerProfileId, followTokenId: followTokenId});
        _followDataByFollowTokenId[followTokenId].profileIdAllowedToRecover = unfollowerProfileId;
    } 

Now, here's the catch. If User A decides to re-follow User B using the same token before recovering it, the system permits this action. The profileIdAllowedToRecover is not reset after a successful re-follow operation, which is evident in the _followByRecoveringToken function:

function _followByRecoveringToken(uint256 followerProfileId, uint256 followTokenId) internal returns (uint256) {
    if (_followDataByFollowTokenId[followTokenId].profileIdAllowedToRecover != followerProfileId) {
        revert FollowTokenDoesNotExist();
    }
    ...
    _baseFollow({followerProfileId: followerProfileId, followTokenId: followTokenId, isOriginalFollow: false});
    return followTokenId;
}

Consequently, User A can keep unfollowing and following User B using the same token (without ever recovering it), leading to an artificial increase in User B's follower count.

The impact of this issue lies in the potential misrepresentation of a user's popularity on the Lens platform. Even though this doesn't cause any direct monetary loss, it could skew the user experience and the system's overall integrity.

Proof of Concept

• User A follows User B, creating a Follow token.
• User A unfollows User B. The Follow token becomes unwrapped, and User A is allowed to recover it.
• Instead of recovering the Follow token, User A follows User B again using the same token.
• The profileIdAllowedToRecover is not reset after the follow operation, so User A is able to unfollow and follow User B again without ever recovering the token.
• By repeating steps 2-4, User A can artificially inflate the follower count of User B.

Recommended Mitigation Steps

To mitigate this issue, the profileIdAllowedToRecover field should be reset after a successful follow operation in the _followByRecoveringToken function. This would prevent a user from being able to follow again with the same token without first recovering it.

function _followByRecoveringToken(uint256 followerProfileId, uint256 followTokenId) internal returns (uint256) {
    if (_followDataByFollowTokenId[followTokenId].profileIdAllowedToRecover != followerProfileId) {
        revert FollowTokenDoesNotExist();
    }
    ...
    _baseFollow({followerProfileId: followerProfileId, followTokenId: followTokenId, isOriginalFollow: false});

    // Reset profileIdAllowedToRecover to default value
    _followDataByFollowTokenId[followTokenId].profileIdAllowedToRecover = 0;

    return followTokenId;
}

In this version of _followByRecoveringToken, the profileIdAllowedToRecover is reset to a default value (0 in this case) after the follow operation has been successfully completed. This change ensures that a user can't follow again with the same token without first recovering it, thus preventing the potential inflation of follower counts.

Assessed type

Other

[donosonaumczuk]

• Follow count can't be unexpecedly increase or decrease by the described mechanism:

  • Follow, even when recovering a token, always increases follower count by one.
  • Unfollowing always decreases follower count by one. The only case where the follower count is outdated, is when a follower profile is burnt. And this was specified in a comment right above the definition of the _followerCount field.

• profileIdAllowedToRecover is cleared upon follow inside the _baseFollow function.

[c4-sponsor]

donosonaumczuk marked the issue as sponsor disputed

[Picodes]

The sponsor's comment looks correct to me, making this report invalid.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid