Users can unfollow through `FollowNFT` contract when LensHub is paused by governance

mediumCode4rena

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L131-L138 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L255-L258

Vulnerability details

Bug Description

When the LensHub contract has been paused by governance (_state set to ProtocolState.Paused), users should not be able unfollow profiles. This can be inferred as the unfollow() function has the whenNotPaused modifier:

LensHub.sol#L368-L371

solidity
    function unfollow(uint256 unfollowerProfileId, uint256[] calldata idsOfProfilesToUnfollow)
        external
        override
        whenNotPaused

However, in the FollowNFT contract, which is deployed for each profile that has followers, the removeFollower() and burn() functions do not check if the LensHub contract is paused:

FollowNFT.sol#L131-L138

solidity
    function removeFollower(uint256 followTokenId) external override {
        address followTokenOwner = ownerOf(followTokenId);
        if (followTokenOwner == msg.sender || isApprovedForAll(followTokenOwner, msg.sender)) {
            _unfollowIfHasFollower(followTokenId);
        } else {
            revert DoesNotHavePermissions();
        }
    }

FollowNFT.sol#L255-L258

solidity
    function burn(uint256 followTokenId) public override {
        _unfollowIfHasFollower(followTokenId);
        super.burn(followTokenId);
    }

As such, whenever the system has been paused by governance, users will still be able to unfollow profiles by wrapping their followNFT and then calling either removeFollower() or burn().

Impact

Users are able to unfollow profiles when the system is paused, which they should not be able to do.

This could be problematic if governance ever needs to temporarily pause unfollow functionality (eg. for a future upgrade, or unfollowing functionality has a bug, etc...).

Proof of Concept

The Foundry test below demonstrates how users will still be able to unfollow profiles by calling wrap() and removeFollower(), even after the system has been paused by governance. It can be run with the following command:

forge test --match-test testCanUnfollowWhilePaused -vvv

solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;

import 'test/base/BaseTest.t.sol';

contract Unfollow_POC is BaseTest {
    address targetProfileOwner;
    uint256 targetProfileId;
    FollowNFT targetFollowNFT;

    address follower;
    uint256 followerProfileId;
    uint256 followTokenId;

    function setUp() public override {
        super.setUp();

        // Create profile for target
        targetProfileOwner = makeAddr("Target");
        targetProfileId = _createProfile(targetProfileOwner);

        // Create profile for follower
        follower = makeAddr("Follower");
        followerProfileId = _createProfile(follower);

        // Follower follows target
        vm.prank(follower);
        followTokenId = hub.follow(
            followerProfileId,
            _toUint256Array(targetProfileId),
            _toUint256Array(0),
            _toBytesArray('')
        )[0];
        targetFollowNFT = FollowNFT(hub.getProfile(targetProfileId).followNFT);
    }

    function testCanUnfollowWhilePaused() public {
        // Governance pauses system
        vm.prank(governance);
        hub.setState(Types.ProtocolState.Paused);
        assertEq(uint8(hub.getState()), uint8(Types.ProtocolState.Paused));

        // unfollow() reverts as system is paused
        vm.startPrank(follower);
        vm.expectRevert(Errors.Paused.selector);
        hub.unfollow(followerProfileId, _toUint256Array(targetProfileId));

        // However, follower can still unfollow through FollowNFT contract 
        targetFollowNFT.wrap(followTokenId);
        targetFollowNFT.removeFollower(followTokenId);        
        vm.stopPrank();

        // follower isn't following anymore
        assertFalse(targetFollowNFT.isFollowing(followerProfileId));
    }
}

Recommended Mitigation

All FollowNFT contracts should check that the LensHub contract isn't paused before allowing removeFollower() or burn() to be called. This can be achieved by doing the following:

Add a whenNotPaused modifier to FollowNFT.sol:

solidity
modifier whenNotPaused() {
    if (ILensHub(HUB).getState() == Types.ProtocolState.Paused) {
        revert Errors.Paused();
    }
    _;
}

Use the modifier on removeFollower() and burn():

FollowNFT.sol#L131-L138

diff
-   function removeFollower(uint256 followTokenId) external override {
+   function removeFollower(uint256 followTokenId) external override whenNotPaused {
        // Some code here...
    }

FollowNFT.sol#L255-L258

diff
-   function burn(uint256 followTokenId) public override {
+   function burn(uint256 followTokenId) public override whenNotPaused {
        // Some code here...
    }