Zero collection module can be whitelisted and set to a post, which will then revert all collects and mirrors with PublicationDoesNotExist

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/LensHub.sol#L128-135

Vulnerability details

Impact

In case when zero collection module be white listed and then zero collection module set to a post (done by different actors), its functionality will be partially broken: every collecting and mirroring of it will be reverted with Errors.PublicationDoesNotExist, while getPubType will return its type as a Mirror.

Setting severity to Medium per 'function of the protocol or its availability could be impacted', as either the behavior of rejecting collects/mirrors is desired and to be implemented explicitly, or such a scenario shouldn't exist in a system, i.e. now zero address absence in white list cannot be guaranteed as its setting is manual and possible

Proof of Concept

Collect module isn't checked additionally on init, any white listed address can be set:

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/libraries/PublishingLogic.sol#L308-309

While zero address also can be white listed:

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/LensHub.sol#L128-135

If zero address is white listed and then set as collectModule for a post, a getPointedIfMirror helper function called on it will revert with Errors.PublicationDoesNotExist:

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/libraries/Helpers.sol#L47

This way all attempts to collect and mirror this post will also revert:

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/libraries/InteractionLogic.sol#L108

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/libraries/PublishingLogic.sol#L258

Also, getContentURI view will fail with the same error:

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

And getPubType will return Mirror as a publication type:

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

Recommended Mitigation Steps

Consider prohibiting zero collection module to be white listed.

If rejecting collects/mirrors is desired for a special post type, the corresponding error path can be implemented

[Zer0dot]

This implies governance is a bad actor, and despite it being a side effect, reverting on a zero collect module is acceptable. IMO this is not relevant, though technically the report is valid, we won't be changing anything.

[Zer0dot]

In consistency with some other comments I made, I wonder if marking this as "low" severity makes more sense. Governance being compromised is within acceptable risk parameters.

[0xleastwood]

In consistency with some other comments I made, I wonder if marking this as "low" severity makes more sense. Governance being compromised is within acceptable risk parameters.

I think this is inline with C4's guidelines for a medium severity issue. As such, I'd keep this as is.

[0xleastwood]

This is consistent with other governance related issues.

[Zer0dot]

Sounds fair!