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

0 stars 0 forks source link

Profile owner can set royalties on a follow NFT they don't own. #122

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/cdef6ebc6266c44c7068bc1c4c04e12bf0d67ead/contracts/FollowNFT.sol#L455-L461

Vulnerability details

Impact

Profile owner can drain royalties from follow NFTs they don't own

Proof of Concept

The _beforeRoyaltiesSet() modifier checking only the profile owner instead of the follow NFT owner presents a major vulnerabilities

It allows the profile owner to set royalties on a follow NFT they don't own. For example:

Alice owns Profile NFT #1 Bob owns Follow NFT #100 following Profile #1 Currently, _beforeRoyaltiesSet would allow Alice to set royalties because she owns Profile #1, even though she doesn't own Follow NFT #100.

It prevents the actual follow NFT owner from setting royalties.

Continuing the example:

Bob owns Follow NFT #100 _beforeRoyaltiesSet reverts if Bob tries to set royalties since he doesn't own Profile #1 So the improper owner check prevents the actual NFT owner from configuring royalties while allowing non-owners to configure them if they own the profile.

Tools Used

Manual

Recommended Mitigation Steps

Check msg.sender against ownerOf(followTokenId) instead of the profile owner.

Assessed type

Other

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

c4-pre-sort commented 1 year ago

141345 marked the issue as remove high or low quality report

141345 commented 1 year ago

Profile NFT owner has control on Collection royalties.

Seems designed behavior

donosonaumczuk commented 1 year ago

This is expected behaviour, the profile being followed (aka target profile) is the owner of the collection, so he is the one receiving the royalties and the one allowed to modify their percentage.

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: Invalid