Unbound length of referrers may lead to DOS of publication actions and modules

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-lens/blob/main/contracts/libraries/ValidationLib.sol#L87-L119 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/modules/act/collect/CollectPublicationAction.sol#L109-L127 https://github.com/code-423n4/2023-07-lens/blob/main/contracts/modules/act/collect/base/BaseFeeCollectModule.sol#L262-L295

Vulnerability details

Summary

Publication functions (post/comment/mirror/quote/act) can receive an array of referrals to pass to their modules/actions.

Those modules and actions will iterate and process those referrals on a later stage.

Referral arrays might be big enough that certain modules and actions might fail when they try to process them due to an Out of Gas error, and leading to a DOS of them.

Impact

Publication modules and actions might revert with Out of Gas due to the unbound length of referrals that have to be processed.

Proof of Concept

Publication actions and modules might process referrals on a while loop. If the length of the array is big enough, the function may revert with out of gas during the loop execution.

Here is an example of CollectPublicationAction using a collect module. In this case, the module is called via processCollect() and the internal function _transferToReferrals(), which makes one external call to transfer tokens for each individual referral.

    function _processCollect(
        address collectModule,
        Types.ProcessActionParams calldata processActionParams
    ) private returns (bytes memory) {
        return
            ICollectModule(collectModule).processCollect(
                Types.ProcessCollectParams({
                    publicationCollectedProfileId: processActionParams.publicationActedProfileId,
                    publicationCollectedId: processActionParams.publicationActedProfileId,
                    collectorProfileId: processActionParams.actorProfileId,
                    collectorProfileOwner: processActionParams.actorProfileOwner,
                    transactionExecutor: processActionParams.transactionExecutor,
@>                  referrerProfileIds: processActionParams.referrerProfileIds,
@>                  referrerPubIds: processActionParams.referrerPubIds,
@>                  referrerPubTypes: processActionParams.referrerPubTypes,
                    data: processActionParams.actionModuleData
                })
            );
    }

CollectPublicationAction.sol#L109-L127

    function _transferToReferrals(
        Types.ProcessCollectParams calldata processCollectParams,
        address currency,
        uint256 amount
    ) internal virtual returns (uint256) {
        uint256 referralFee = _dataByPublicationByProfile[processCollectParams.publicationCollectedProfileId][
            processCollectParams.publicationCollectedId
        ].referralFee;
        uint256 totalReferralsAmount;
        if (referralFee != 0) {
            // The reason we levy the referral fee on the adjusted amount is so that referral fees
            // don't bypass the treasury fee, in essence referrals pay their fair share to the treasury.
            totalReferralsAmount = (amount * referralFee) / BPS_MAX;
            uint256 numberOfReferrals = processCollectParams.referrerProfileIds.length;
            uint256 amountPerReferral = totalReferralsAmount / numberOfReferrals;
            if (amountPerReferral > 0) {
                uint256 i;
@>              while (i < numberOfReferrals) {
@>                  address referralRecipient = IERC721(HUB).ownerOf(processCollectParams.referrerProfileIds[i]);

                    // Send referral fee in ERC20 tokens
                    IERC20(currency).safeTransferFrom(
                        processCollectParams.transactionExecutor,
@>                      referralRecipient,
                        amountPerReferral
                    );
                    unchecked {
                        ++i;
                    }
                }
            }
        }
        return amount - totalReferralsAmount;
    }

BaseFeeCollectModule.sol#L262-L295

Referrals are passed to PublicationLib and ActionLib functions, and validated via ValidationLib.

The ValidationLib::validateReferrersAndGetReferrersPubTypes() function validates that referrerProfileIds and referrerPubIds have the same length, but it actually doesn't check that the length of the referrals are below a certain limit:

    function validateReferrersAndGetReferrersPubTypes(
        uint256[] memory referrerProfileIds,
        uint256[] memory referrerPubIds,
        uint256 targetedProfileId,
        uint256 targetedPubId
    ) internal view returns (Types.PublicationType[] memory) {
        if (referrerProfileIds.length != referrerPubIds.length) {
            revert Errors.ArrayMismatch();
        }
@>      Types.PublicationType[] memory referrerPubTypes = new Types.PublicationType[](referrerProfileIds.length);

        // We decided not to check for duplicate referrals here due to gas cost. If transient storage opcodes (EIP-1153)
        // get included into the VM, this could be updated to implement an efficient duplicate check mechanism.
        // For now, if a module strongly needs to avoid duplicate referrals, it can check for them at its own expense.

        uint256 referrerProfileId;
        uint256 referrerPubId;
        uint256 i;
        while (i < referrerProfileIds.length) {
            referrerProfileId = referrerProfileIds[i];
            referrerPubId = referrerPubIds[i];
            referrerPubTypes[i] = _validateReferrerAndGetReferrerPubType(
                referrerProfileId,
                referrerPubId,
                targetedProfileId,
                targetedPubId
            );
            unchecked {
                i++;
            }
        }
        return referrerPubTypes;
    }

ValidationLib.sol#L87-L119

Tools Used

Manual Review

Recommended Mitigation Steps

Limit the number of referrerProfileIds and referrerPubIds to a max cap on ValidationLib::validateReferrersAndGetReferrersPubTypes().

Assessed type

Invalid Validation

[141345]

act() -> processPublicationAction() -> _processCollect() this could affect the entire operation.

[donosonaumczuk]

That's ok, we didn't limit the amount at protocol level because it's already limited by the Gas. Users will decide how many actions to set, and which ones, based on their budget. API and relayers will probably limit the amount of actions they allow, so they don't overspend when executing a meta-tx on behalf ot a user.

We don't see this as an issue.

[c4-sponsor]

donosonaumczuk marked the issue as sponsor disputed

[Picodes]

Low severity as long as it's Users inputting these values or API and relayers executing job as it's within their job to not overspend

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)