code-423n4 / 2022-02-aave-lens-findings

0 stars 0 forks source link

Need a way to hide/disable posts #43

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/aaf6c116345f3647e11a35010f28e3b90e7b4862/contracts/core/LensHub.sol#L486-L496

Vulnerability details

It is important for any social network to allow its users to delete posts that they no longer wish to be visible, whether it be due to a typo, account compromise, or no longer holding the same views as before. While information written to the blockchain is forever, the public interface to the social network need not make all such access available.

Impact

If someone has a typo in their URI or otherwise wishes to remove a post, the platform provides no method of doing so. Their only option is to create a new profile and burn() the old one. A user shouldn't have to create a new profile and lose all followers and linkages just because of a typo.

Proof of Concept

    /**
     * @notice Burns a profile, this maintains the profile data struct, but deletes the
     * handle hash to profile ID mapping value.
     *
     * NOTE: This overrides the LensNFTBase contract's `burn()` function and calls it to fully burn
     * the NFT.
     */
    function burn(uint256 tokenId) public override whenNotPaused {
        super.burn(tokenId);
        _clearHandleHash(tokenId);
    }

https://github.com/code-423n4/2022-02-aave-lens/blob/aaf6c116345f3647e11a35010f28e3b90e7b4862/contracts/core/LensHub.sol#L486-L496

Tools Used

Code inspection

Recommended Mitigation Steps

Allow the user to mark posts as no longer visible

oneski commented 2 years ago

declined. This is as designed. Moderation and deletion is done on the FE level not within the contracts.

This was an intentional design choice by the team.

0xleastwood commented 2 years ago

How would this be handled on the front-end level? Are posts flagged as deleted in the smart contract and this is reflected in the front-end? Or are all front-ends meant to hold a shared state on what posts are deleted. @oneski

Zer0dot commented 2 years ago

So there's nothing stopping a canonical source of truth in a contract from being built (which is kinda cool) but originally the idea is to delegate this responsibility to the FE entirely. In other words, they should hold their own state.

0xleastwood commented 2 years ago

If we consider the possibility that there will be multiple front-ends holding their own state. How can we be sure that states are consistent between front-ends? It seems like to me that this is one case where on-chain enforcement is actually ideal.

Zer0dot commented 2 years ago

I actually agree, I'll bring it up internally-- we could have a similar system to the "toggle follow" mechanism we've implemented. This would consist in simply emitting an event signifying whether or not you want to "hide" that publication.

0xleastwood commented 2 years ago

I think there is benefit to having both as an option. Users can opt to do it on-chain if they want better enforcement between front-ends. But I think its potentially dangerous to expect front-ends to responsibly handle their own states.

0xleastwood commented 2 years ago

I think in light of our discussion, it would be best to leave this as medium risk.

oneski commented 2 years ago

gm @0xleastwood personally disagree, that it is the role of the base contracts to handle such behaviour, a Lens post by default is supposed to be immutable, privacy and the ability to delete can be added by a user via a higher level protocol such as Lit or Unlock protocol. in the same way IPFS is immutable even if you unpin, Lens should act the same way.

0xleastwood commented 2 years ago

Hmm okay, I think I'm inclined to side with you on this one. This seems to be inline with other protocols.