Minting batch tokens erroneously burns ETH from caller

Summary

When minting batch tokens from the Edition contract, the FeeManager contract is called to collect the fees based on the amount of ETH sent by the caller. In the case of batch minting, referrer address is set to the zero address, indicating that only protocol and fee receiver should receive fees. However, a portion of the ETH sent by the caller is still sent to the referrer address, which is the zero address, effectively burning them.

Vulnerability Detail

Edition::mintBatch #L277-297

    function mintBatch(
        address to_,
        uint256[] calldata tokenIds_,
        uint256[] calldata amounts_,
        bytes calldata data_
    ) external payable {
        for (uint256 i = 0; i < tokenIds_.length; i++) {
            Work storage work = works[tokenIds_[i]];

            // @audit referrer is set to zero address
@>          FEE_MANAGER.collectMintFee{value: msg.value}(
                this, tokenIds_[i], amounts_[i], msg.sender, address(0), work.strategy
            );

            _checkTime(work.opensAt, work.closesAt);
            _updateSupply(work, amounts_[i]);
        }

        _batchMint(to_, tokenIds_, amounts_, data_);
        _refundExcess();
    }

A call to FeeManager::collectMintFee is made with referrer address set to the zero address.

FeeManager::collectMintFee #L366-L410

    function collectMintFee(
        IEdition edition,
        uint256 tokenId_,
        uint256 amount_,
        address payer_,
        address referrer_,
        Strategy calldata strategy_
    ) external payable {
        _collectMintFee(
            edition, tokenId_, amount_, payer_, referrer_, getMintFee(strategy_, amount_)
        );
    }

    function _collectMintFee(
        IEdition edition_,
        uint256 tokenId_,
        uint256 amount_,
        address payer_,
        address referrer_,
        Fee memory fee_
    ) internal {
        if (fee_.amount == 0) return;

        .
        .
        .

        uint256 protocolFee = protocolFlatFee * amount_;
        uint256 protocolShare;
        if (fee_.amount == protocolFee) {
            protocolShare = protocolFee * protocolFeeshareBps / MAX_BPS;
        } else {
            protocolShare = protocolFee;
        }
  // @audit fees are sent to fee receiver
        _route(
            Fee({asset: fee_.asset, amount: fee_.amount - protocolShare}),
            _feeReceivers[getRouteId(edition_, tokenId_)],
            payer_
        );
// @audit referrer is sent fees here
        uint256 referrerShare =
@>          _splitProtocolFee(edition_, fee_.asset, protocolShare, payer_, referrer_);
        emit FeeCollected(address(edition_), tokenId_, fee_.asset, fee_.amount, referrerShare);
    }

FeeManager::_splitProtocolFee #L412-L441

    function _splitProtocolFee(
        IEdition edition_,
        address asset_,
        uint256 amount_,
        address payer_,
        address referrer_
    ) internal returns (uint256 referrerShare) {
        // @audit this will be zero since referrer address is set to zero address
        uint256 mintReferrerShare = getMintReferrerShare(amount_, referrer_);

        // @audit this will be non-zero since `referrers[edition_]` is not set to the zero address during this call
        uint256 collectionReferrerShare = getCollectionReferrerShare(amount_, referrers[edition_]);
        referrerShare = mintReferrerShare + collectionReferrerShare;

        _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_
        );

  //@audit target is set to the zero address with the amount collectionReferrerShare > 0
@>      _route(
            Fee({asset: asset_, amount: collectionReferrerShare}),
            Target({target: referrer_, chainId: block.chainid}),
            payer_
        );
    }

collectionReferrerShare > 0, since that amount is retrieved from referrers[edition_], which in this call is not set to address 0. collectionReferrerShare is sent to address 0 since we set target to referrer_, effectively burning it:

FeeManager::_route #L448-L454

    function _route(Fee memory fee_, Target memory feeReceiver_, address feePayer_) internal {
        if (block.chainid != feeReceiver_.chainId) revert NotRoutable();
        if (fee_.amount == 0) return;
           // @audit feeReceiver_.target is the zero address in this case
@>      _transfer(fee_.asset, fee_.amount, feePayer_, feeReceiver_.target);
    }

FeeManager::_transfer #L461-L467

    function _transfer(address asset_, uint256 amount_, address from_, address to_) internal {
        if (asset_ == ETH_ADDRESS) {
@>          to_.safeTransferETH(amount_);
        } else {
            asset_.safeTransferFrom(from_, to_, amount_);
        }
    }

Impact

Loss of funds due to erroneously burning ETH

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L277-L297

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L412-L441

Tool used

Manual Review

Recommendation

Check if referrer address is set to the zero address prior to routing

sherlock-audit / 2024-04-titles-judging

cryptic - Minting batch tokens erroneously burns ETH from caller #216