Reduction of the commission for the mint due to a lack of data validation in the _collectMintFee function

Summary

The lack of data validation in the FeeManager::calculateMintFee and FeeManager::_calculateMintFee functions has been described in detail in this issue Here I would like to focus on a specific edge case, when msg.sender is specified as the referer_

If you specify yourself as a referer, then 50% of the protocolFee will be sent to your address in the FeeManager::_splitProtocolFee function, thus you significantly reduce the cost of the commission you pay for the mint.

Vulnerability Detail

I have described the process of paying commissions in minutes in detail in the other two issues first second first Title - In the FeeManager.sol::_splitProtocolFee function, the collectionReffererShare recipient is misspelled. second Title - There is no validation on the FeeManager::collectMintFee function, anyone can call it It can be seen from them that the specified refferer_ is not validated in any way and receives 50% of the protocolFee

Impact

The commission fee is greatly reduced for the user who takes advantage of this last resort. Score: medium

Code Snippet

_splitProtocolFee function

function _splitProtocolFee(
        IEdition edition_,
        address asset_,
        uint256 amount_,
        address payer_,
        address referrer_
    ) internal returns (uint256 referrerShare) {
        // The creation and mint referrers earn 25% and 50% of the protocol's share respectively, if applicable
        uint256 mintReferrerShare = getMintReferrerShare(amount_, referrer_);
        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_
        );

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

Tool used

Manual Review

Recommendation

Add appropriate validation to eliminate this edge case

Duplicate of #405

sammy-tm commented 1 month ago

Escalate

sherlock-admin3 commented 1 month ago

Escalate

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.

BengalCatBalu commented 1 month ago

Here I dealt with the case where the buyer himself is listed as the refferer. _calculateMintFee does not validate this case in any way, although it in turn allows to reduce paid commissions by 50%. (50% goes to the refferer, because it is assumed that the nft is made on his template or something, but there is no way to control that the refferer != msg.sender) I didn't specify this in the report, unfortunately, but it also adds an extra risk of potentially Reentrancy.

I think this is a valid issue, as the problem with unnecessary user power when specifying refferer can lead to a reduced commission. The protocol should automatically calculate the refferer using Graph

WangSecurity commented 1 month ago

Agree with the escalation, planning to accept and duplicate with #267

Evert0x commented 1 month ago

Result: High Duplicate of #267

sherlock-admin4 commented 1 month ago

Escalations have been resolved successfully!

Escalation status:

sammy-tm: accepted