Aave Lens contest - gzeon's results

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/FeeCollectModule.sol#L99

Vulnerability details

Impact

User can refer themselves when collect in any CollectModule that collect fee. The will lead to value leak as user can always refer themselves to receive a referral fee as discount.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/collect/FeeCollectModule.sol#L99

Recommended Mitigation Steps

Check if referrer == collector. Alternatively, only allow existing collectors to earn referral fee to incentivize.

LensHub constructor lack input validation

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L57 Recommended to add check to make sure addresses supplied is not address(0)

CollectNFT constructor lack input validation

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/CollectNFT.sol#L29 Recommended to add check to make sure addresses supplied is not address(0)

FollowNFT constructor lack input validation

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L48 Recommended to add check to make sure addresses supplied is not address(0)

Interdependency of LensHub and CollectNFT/FollowNFT construction

CollectNFT/FollowNFT require the address of LensHub in the constructor, while LensHub require the address of CollectNFT and FollowNFT. While we can use a pre-computed address, it might be better to call the initializer from LensHub to set everything at once.

LensHub.initialize lack input validation

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L63 Recommended to add check to make sure newGovernance is not address(0)

Add comment to explain why ++pubCount is not used

The way _createComment coded is likely to prevent comment loop so pubCount is incremented after PublishingLogic.createComment. It looks a bit werid since other function use ++pubCount instead. Recommed to add soe comment here for future reference. https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L878

    function _createComment(DataTypes.CommentData memory vars) internal {
        PublishingLogic.createComment(
            vars,
            _profileById[vars.profileId].pubCount + 1,
            _profileById,
            _pubByIdByProfile,
            _collectModuleWhitelisted,
            _referenceModuleWhitelisted
        );
        _profileById[vars.profileId].pubCount++;
    }

Implement 2 steps governance transfer in LensHub

To reduce risk of failed governance transfer, use a 2 steps process that require the new governance's interaction. https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L78

Implement 2 steps governance transfer in ModuleGlobals

To reduce risk of failed governance transfer, use a 2 steps process that require the new governance's interaction. https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/modules/ModuleGlobals.sol#L50

Use uint256 instead of bool

Use uint(1) instead of bool(true) to save gas by avoiding masking ops https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/storage/LensHubStorage.sol#L59

    mapping(address => bool) internal _profileCreatorWhitelisted;
    mapping(address => bool) internal _followModuleWhitelisted;
    mapping(address => bool) internal _collectModuleWhitelisted;
    mapping(address => bool) internal _referenceModuleWhitelisted;

Unchecked increment

It is virtually impossible for these counter to overflow https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/LensHub.sol#L148

        uint256 profileId = ++_profileCounter;

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/CollectNFT.sol#L51

        uint256 tokenId = ++_tokenIdCounter;

https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/core/FollowNFT.sol#L58

        uint256 tokenId = ++_tokenIdCounter;

_validateHandle gas optimization

'.' must be < '0' https://github.com/code-423n4/2022-02-aave-lens/blob/c1d2de2b0609b7d2734ada2ce45c91a73cc54dd9/contracts/libraries/PublishingLogic.sol#L398

    function _validateHandle(string calldata handle) private pure {
        bytes memory byteHandle = bytes(handle);
        if (byteHandle.length == 0 || byteHandle.length > Constants.MAX_HANDLE_LENGTH)
            revert Errors.HandleLengthInvalid();

        for (uint256 i = 0; i < byteHandle.length; ++i) {
            if (
                ((byteHandle[i] < '0' && byteHandle[i] != '.') ||
                    byteHandle[i] > 'z' ||
                    (byteHandle[i] > '9' && byteHandle[i] < 'a'))
            ) revert Errors.HandleContainsInvalidCharacters();
        }
    }

createProfile gas optimization

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

        bytes memory followModuleReturnData;
        if (vars.followModule != address(0)) {
            _profileById[profileId].followModule = vars.followModule;
            followModuleReturnData = _initFollowModule(
                profileId,
                vars.followModule,
                vars.followModuleData,
                _followModuleWhitelisted
            );
        }else{
            followModuleReturnData = new bytes(0);
        }

        _emitProfileCreated(profileId, vars, followModuleReturnData);
    }