The user who is about to be followed will frontrun a follower with a `LensHub#setFollowModule` call to steal from them.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/LensHub.sol#L129

Vulnerability details

Impact

The power granted to users to freely set followModule at anytime allows for potential exploitation by frontrunning and manipulating fees, resulting in users unintentionally paying high fees when they intended to follow others for free or at a minimal cost

Proof of Concept

User that wanted to pay 0 or little fee to follow a particular user can end up paying very high fees. Here are some attack screnarios Example 1

• Charles, a user on the platform, sets a fee of 100 USDC for anyone who wants to follow him.

• Alice, who seeks to deceive others, ensures that her profile has no followModule attached to it, making it appear as if it's free to follow her.

• Bob, an innocent user, decides to follow Charles and grants approval to the FeeFollowModule, allowing it to spend up to 1000 USDC. It's common for users to approve more tokens than needed as a safety measure and better user experience

• Now, Bob, having already spent on following Charles, wishes to follow Alice, who appears to be free to follow. Little does Bob know that he's about to fall into a trap.

• In a devious move known as frontrunning( by increasing gas price of a transaction), Alice utilizes the setFollowModule function to set her profile's followModule to the very same FollowModule that Bob had previously approved. Additionally, she sets her own fee to the minimum value between Bob's approved followModule amount and the USDC remaining in Bob's wallet.

• As a result, Alice cunningly drains the tokens that Bob had approved, taking advantage of Bob's initial intention not to spend anything on following her.

Example 2

• Alice sets a fee of 100 USDC for users who wish to follow her.

• Bob decides to follow Alice and approves 1000 USDC for the followModule attached to Alice's profile.

• Alice uses the setFollowModule function to change the fee for following her to 1000 USDC, aligning it with the amount Bob approved.

• As a result of Alice's deceitful actions, Bob ends up paying more tokens than he originally expected, falling victim to Alice's malicious scheme.

Tools Used

Manual Review

Recommended Mitigation Steps

There could be other better ways of fixing this, but here is my suggestion:

• In ProfileLib#setFollowModule function, the parameters and current block number should be cached but _initFollowModule should not be called yet:

  function setFollowModule(uint256 profileId, address followModule, bytes calldata followModuleInitData) external {
  Types.Profile storage userProfile=StorageLib.getProfile(profileId);
  userProfile.followModule = followModule;
  userProfile.cachedBlockNumber=block.number;
  userProfile.followModuleInitData=followModuleInitData;
  
  //emit Events.FollowModuleSet(profileId, followModule, followModuleReturnData, block.timestamp);
  }

• In FollowLib#_follow function, if cachedBlockNumber!=0 && currentBlockNumber>cachedBlockNumber, _initFollowModule should be called with the cached parameters, then cached block number for the profile should be set to 0:

  
  function _follow(
      uint256 followerProfileId,
      address transactionExecutor,
      uint256 idOfProfileToFollow,
      uint256 followTokenId,
      bytes calldata followModuleData
  ) private returns (uint256) {
      ...
      uint256 followTokenIdAssigned = IFollowNFT(followNFT).follow({
          followerProfileId: followerProfileId,
          transactionExecutor: transactionExecutor,
          followTokenId: followTokenId
      });

//code block to add start @> if ((_profileToFollow.cachedBlockNumber!=0) && (block.number>cachedBlockNumber)){ @> // perform ProfileLib#_initFollowModule logic here with cached followModule and followModuleInitData as parameters @> } //code block to add end

    bytes memory processFollowModuleReturnData;
    ...

}

However, the drawback of this approach is that the next follower after setFollowModule is called will have to pay extra gas. Additionally, the owner of the profile being followed can potentially use setFollowModule frequently to inconvenience users (although this scenario is unlikely to happen in reality, as there will be no gain from inconveniencing one's own followers).

## Assessed type

Timing

[141345]

timelock issue

QA might be more appropriate.

[donosonaumczuk]

We are aware of this, and that's why the modules require whitelisting to be applied, so they cannot hold malicious behaviour. All modules involving fees expect custom data which contains currency and amount, so these kind of attacks cannot be performed successfully. In conclusion, this does not apply for current state of the protocol.

The solution proposed it's really interesting in case of Lens Protocol removing the whitelist at some point. Props for that!

[donosonaumczuk]

We misunderstood the solution proposed, and actually we think it's not a good idea. What we think can work for a context where Lens does not have whitelist for modules anymore is:

• When a follow module is changed, it is initialized and the block number is saved.
• Follower needs to pass the expected "previous follow module change" block number. The follow only succeeds if the block number matches. This can only have issues if UIs are malicious, which is a more acceptable risk, as the UI can also make you perform a follow an incorrect follwo anyways.

[donosonaumczuk]

We keep current logic as it is. This issue does not apply for current state of Lens Protocol, so we dispute its vaildity.

[c4-sponsor]

donosonaumczuk marked the issue as sponsor disputed

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

Downgrading to QA as the report is valuable, although not of high severity as long as there is a whitelisting system

[c4-judge]

Picodes marked the issue as grade-b