code-423n4 / 2023-07-lens-findings

0 stars 0 forks source link

unfollow() functionality allows the owner to reserve the tokenId but not follow the owner of the FollowNFT collection #101

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Let's say the follower is Alice and the owner of the FollowNFT collection is Bob.

In FollowNFT.sol, unfollow() function is called by Alice to unfollow Bob and clean all the storage. The problem is that the wrapped token always stays with the follower. Even when the user is blocked by the creator, it first mints ERC721 token and only after that _unfollow() is called. So technically, Alice is not following Bob anymore as all the state is cleaned. For example, when Alice is blocked by Bob, she can't follow() him anymore. But if she just calls unfollow(), she can then again follow() with the same tokenId. This becomes a serious concern when Bob is a content creator and he gives some premium content for the users with tokenIds from 0 to 10, for example. This way, if Alice, say, has followTokenId of 5, she may not follow Bob anymore but she still holds the token and she basically reserved this followTokenId. New users will not be able to get it as the token is not burnt when calling unfollow(). So, other than Alice, nobody can burn this token. When newcomers call follow(), they have to put 0 as followTokenId param and the followTokenIdAssigned will be just +1 from the last tokenId minted. That being said, if Alice is being blocked (or just unfollows), her tokenId is still among the number of the followers that get premium content, and nobody can get her followTokenId unless she burns hers. Bob has no control over this situation and he may give his content to the users such as Alice that are not following him or even blocked.

It doesn't have to be necessarily some content, any perks can be given by the owner of the collection to his followers, and this is usually attached to having certain followTokenIds as the right to get the privileges. Thus, it's worth mentioning that, oftentimes, in the case if Bob has a huge fanbase, this problem can particularly affect the owners of the first followTokenIds (let's say, the first 100). These tokenIds are the most likely to get some premium from the owner of the collection and if follower doesn't follow() anymore or is blocked, this "following power" provided by owning the certain followTokenId is lost as his tokenId number remains with him.

Proof of Concept

unfollow():

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

internal _unfollow() that doesn't burn wrapped token:

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

minting wrapped token before calling _unfollow() in processBlock():

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

when calling follow(), if _unsafeOwnerOf() of followTokenId != address(0), _followWithWrappedToken() will be called so that it always has to be 0 as followTokenId argument in this function and new tokenId will be minted with the number that is +1 to the last tokenId that was minted:

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

Tools Used

Manual review.

Recommended Mitigation Steps

Implement the logic so that the user will not reserve the tokenId while not being a follower and not allowing others to get this particular tokenId.

Assessed type

Other

141345 commented 1 year ago

seems expected behavior, no significant loss

donosonaumczuk commented 1 year ago

It is like this by design. More complex mechanisms can be added in the future, but for now we wanted it to work like it is.

c4-sponsor commented 1 year ago

donosonaumczuk marked the issue as sponsor disputed

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Insufficient proof