Closed code423n4 closed 1 year ago
141345 marked the issue as primary issue
We confirm the issue, although this might be rather considered as Low instead of Medium
vicnaum marked the issue as disagree with severity
Picodes marked the issue as satisfactory
Downgrading to Low as assets nor availability of the protocol is at risk
Picodes changed the severity to QA (Quality Assurance)
Hi @Picodes. I'd like to ask if you could take a second look at this issue as a Medium risk finding, considering the Docs:
2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted
In this case the "function of the protocol" is impacted, and availability can also be considered as well. Not for being unavailable, but because of functions being available in moments that they shouldn't be, allowing important actions for a social network that users should not be able to perform, out of the control of the protocol.
When the protocol is paused, it should not allow unfollow actions (via removeFollower()
) or new follows (via batchMigrateProfiles()
for example, among all the other mentioned functions on the Impact section.
In DeFi protocols, missing pause modifiers having been evaluated as Medium like here, and here. In the case of Lens Protocol, the functions mentioned on the Impact section should be considered of ultimate importance to have under control as a social network.
Hi @Picodes, just wanted to mention that #144 is a duplicate of this issue in case it is upgraded. Thanks!
Yeah, I think it is a fair argument and can be upgraded to Medium.
My view on this is that it ultimately depends on the sponsor's intent. In this case, it seems clear by the above comment and what you highlighted that the intent was to be able to totally pause follows and unfollows, so you're right and I'll upgrade this to Med as a functionality is broken
This previously downgraded issue has been upgraded by Picodes
Picodes marked issue #144 as primary and marked this issue as a duplicate of 144
Lines of code
https://github.com/code-423n4/2023-07-lens/blob/main/contracts/LensHub.sol#L368-L373 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/misc/LensV2Migration.sol#L33 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/misc/LensV2Migration.sol#L37-L41 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/misc/LensV2Migration.sol#L45 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/base/LensProfiles.sol#L93-L98 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/base/LensProfiles.sol#L165-L169 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L156 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L164 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L188 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L255 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/FollowNFT.sol#L436-L440
Vulnerability details
Summary
Important user functions on Lens V2 are protected by pause modifiers like
whenNotPaused
, orwhenPublishingEnabled
.This includes functions to follow, unfollow profiles, set follow modules, or create profiles among others.
Impact
Several important functions that modify contract storage by users, or perform actions on tokens are not protected by any pause modifiers. This allows users to interact with the contracts, although they should not be supposed to.
The most important ones being:
FollowNFT::removeFollower()
despiteLensHub::unfollow
is protected withwhenNotPaused
LensHub
via:LensV2Migration::batchMigrateProfiles()
LensV2Migration::batchMigrateFollows()
LensV2Migration::batchMigrateFollowModules()
In addition, some other
FollowNFT
functions could be considered for pause modifiers, asLensHub
tokens have similar protections on the inheritedLensProfiles
contract:FollowNFT::wrap()
FollowNFT::unwrap()
FollowNFT::burn()
FollowNFT::_beforeTokenTransfer()
Proof of Concept
Unfollow actions are prevented on paused contracts for
LensHub::unfollow()
, but can still be executed viaFollowNFT::removeFollower()
:LensHub.sol#L368-L373
FollowNFT.sol#L131-L138
LensV2Migration
is inherited byLensHub
and has the following functions missing any pause modifiers:LensProfiles
is inherited byLensHub
and protects certain functions regarding the token like these ones:On the other hand,
FollowNFT
has no pause protection on functions related to its token. Such as:Tools Used
Manual Review
Recommended Mitigation Steps
Add the corresponding pause modifiers to the functions mentioned on the Impact section.
Assessed type
Other