code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

When a user creates a publication, _initPubCollectModule is used to initialize CollectModule, but when collectModule is 0, subsequent calls to initializePublicationCollectModule for 0 address will fail even if the 0 address is in _collectModuleWhitelisted.

    function _initPubCollectModule(
        uint256 profileId,
        uint256 pubId,
        address collectModule,
        bytes memory collectModuleData,
        mapping(uint256 => mapping(uint256 => DataTypes.PublicationStruct))
            storage _pubByIdByProfile,
        mapping(address => bool) storage _collectModuleWhitelisted
    ) private returns (bytes memory) {
        if (!_collectModuleWhitelisted[collectModule]) revert Errors.CollectModuleNotWhitelisted();
        _pubByIdByProfile[profileId][pubId].collectModule = collectModule;
        return
            ICollectModule(collectModule).initializePublicationCollectModule(
                profileId,
                pubId,
                collectModuleData
            );
    }

Proof of Concept

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

Tools Used

None

Recommended Mitigation Steps

    function _initPubCollectModule(
        uint256 profileId,
        uint256 pubId,
        address collectModule,
        bytes memory collectModuleData,
        mapping(uint256 => mapping(uint256 => DataTypes.PublicationStruct))
            storage _pubByIdByProfile,
        mapping(address => bool) storage _collectModuleWhitelisted
    ) private returns (bytes memory) {
+if (collectModule != address(0)) {
        if (!_collectModuleWhitelisted[collectModule]) revert Errors.CollectModuleNotWhitelisted();
        _pubByIdByProfile[profileId][pubId].collectModule = collectModule;
        return
            ICollectModule(collectModule).initializePublicationCollectModule(
                profileId,
                pubId,
                collectModuleData
            );
    }
+} else {
+            return new bytes(0);
+        }

Zer0dot commented 2 years ago

Invalid. You are not supposed to be able to set the collect module as the zero address anyway since it interferes with logic for determining parent and child content (mirrors don't have collect modules).

0xleastwood commented 2 years ago

I agree, I don't see why the collect module would be set to the zero address.

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