ast3ros - Users can create work and mint without fees

[sherlock-admin3]

ast3ros

medium

Users can create work and mint without fees

Summary

The contract does not refund excess funds to the feePayer; instead, it retains them within the FeeManager. Consequently, if excess funds are present from another user's transaction, users can potentially create works or mint without incurring fees.

Vulnerability Detail

When a new work is published, the feePayer transmits ETH to the TitlesCore contract, which forwards all received funds to the FeeManager. If the transmitted amount exceeds the required fee, the excess is not returned to the feePayer but stored in the FeeManager. This issue allows for fee-free creation and minting if sufficient excess balance is present.

    function _publish(Edition edition_, WorkPayload memory work_, address referrer_)
        internal
        returns (uint256 tokenId)
    {
        ...

        // Collect the creation fee
        // wake-disable-next-line reentrancy
        feeManager.collectCreationFee{value: msg.value}(edition_, tokenId, msg.sender); // @audit no exceed amount returned

        ...
    }

https://github.com/sherlock-audit/2024-04-titles/blob/2ac20d07d0b4562c0b0ee15e1becbf786f0ed896/wallflower-contract-v2/src/TitlesCore.sol#L120-L149

In the process of collecting creation and minting fees, the FeeManager calculates the fee and directs the specified ETH amount from its balance to the intended recipients. It does not verify whether the fee amount was actually paid by the feePayer, leading to potential exploitation.

    function collectCreationFee(IEdition edition_, uint256 tokenId_, address feePayer_)
        external
        payable
    {
        Fee memory fee = getCreationFee();
        if (fee.amount == 0) return;

        _route(fee, Target({target: protocolFeeReceiver, chainId: block.chainid}), feePayer_);
        emit FeeCollected(address(edition_), tokenId_, ETH_ADDRESS, fee.amount, 0);
    }

        function _collectMintFee(
        IEdition edition_,
        uint256 tokenId_,
        uint256 amount_,
        address payer_,
        address referrer_,
        Fee memory fee_
    ) internal {
        ...
        _route(
            Fee({asset: fee_.asset, amount: fee_.amount - protocolShare}),
            _feeReceivers[getRouteId(edition_, tokenId_)],
            payer_
        );

        uint256 referrerShare =
            _splitProtocolFee(edition_, fee_.asset, protocolShare, payer_, referrer_);
        emit FeeCollected(address(edition_), tokenId_, fee_.asset, fee_.amount, referrerShare);
    }

    function _splitProtocolFee(
        IEdition edition_,
        address asset_,
        uint256 amount_,
        address payer_,
        address referrer_
    ) internal returns (uint256 referrerShare) {
        ...

        _route(
            Fee({asset: asset_, amount: amount_ - referrerShare}),
            Target({target: protocolFeeReceiver, chainId: block.chainid}),
            payer_
        );

        _route(
            Fee({asset: asset_, amount: mintReferrerShare}),
            Target({target: referrer_, chainId: block.chainid}),
            payer_
        );

        _route(
            Fee({asset: asset_, amount: collectionReferrerShare}),
            Target({target: referrer_, chainId: block.chainid}),
            payer_
        );
    }

    /// @notice Routes the given {Fee} to the appropriate receiver.
    /// @param fee_ The {Fee} to route.
    /// @param feeReceiver_ The {Target} to receive the fee.
    /// @param feePayer_ The address of the account paying the fee.
    /// @dev If the fee amount is zero, this function will return early. If the receiver is not on the same chain as the payer, this function will revert.
    function _route(Fee memory fee_, Target memory feeReceiver_, address feePayer_) internal {
        // Cross-chain fee routing is not supported yet
        if (block.chainid != feeReceiver_.chainId) revert NotRoutable();
        if (fee_.amount == 0) return;

        _transfer(fee_.asset, fee_.amount, feePayer_, feeReceiver_.target); // @audit send directly from FeeManger balance, no check if the amount is actually sent by the feePayer
    }

https://github.com/sherlock-audit/2024-04-titles/blob/2ac20d07d0b4562c0b0ee15e1becbf786f0ed896/wallflower-contract-v2/src/fees/FeeManager.sol#L366-L454

Impact

If excess funds remain in the FeeManager from another transaction, it enables users to mint or create works without fees. This could potentially lead to unintended free usage of the platform.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/2ac20d07d0b4562c0b0ee15e1becbf786f0ed896/wallflower-contract-v2/src/TitlesCore.sol#L120-L149 https://github.com/sherlock-audit/2024-04-titles/blob/2ac20d07d0b4562c0b0ee15e1becbf786f0ed896/wallflower-contract-v2/src/fees/FeeManager.sol#L366-L454

Tool used

Manual Review

Recommendation

• Refund the remaining amount to the feePayer if the amount is more than the fee when publishing work.
• Check if the amount of the fee is actually sent by the feePayer when collecting the creation and minting fees.

[thangtranth]

Escalate

This is a valid issue. Please help to check.

[sherlock-admin3]

Escalate

This is a valid issue. Please help to check.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Hash01011122]

@thangtranth Please elaborate on why do you think this is valid

[WangSecurity]

All escalations have to have reasoning, but this report doesn't explain why this report should be valid. Hence, it should be rejected. Moreover, in comments for issue #30 it's explained why reports about not refunding excess fees for creation are invalid.

Planning to reject the escalation and leave the issue as it is.

[Evert0x]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• thangtranth: rejected