sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

ArsenLupin - During the collectMintFee the collection referrer doesn't receive any fees. #370

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 5 months ago

ArsenLupin

high

During the collectMintFee the collection referrer doesn't receive any fees.

Summary

The fee transfer works incorrectly, the collection referrer doesn't receive any fees, because his part goes to the mint referrer (arbitrary address)

Vulnerability Detail

During the collectMintFee the collection referrer receives any fees. Mint referrer receive collection referrer fees instead. The main part here, that during the collectMintFee, the _splitProtocolFee is called. This function is responsible for distribute the fees between the

  1. protocolFeeReceiver
  2. collection referrer
  3. mint referrer However, the function works incorrectly and sends the fees that belong to the collection referrer to the mint referrer.

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

    The referrer in our case is mintRefferer, while the collection referrer is stored in the referrers[edition]. We could prove it by take a look how the collectionFee is calculated

uint256 collectionReferrerShare = getCollectionReferrerShare(amount_, referrers[edition_]);

Proof of Concept

function setUp() public {
        feeManager = new FeeManager(address(1), address(this), address(new MockSplitFactory()));
    }

function test_collectMintFeeBug() public {
        /** PoC 
         * 1. Deploy the feeManager
         * 2. SetUp the attributions/allocations/splitFactory. It is done so the creator and attributors could receive the fees.
         * 3. CreateRoute. It is done to set a collection referrer, who refers the creation of a new Edition, and gets a cut of all mint fees for that Edition
         * 4. Create a mint refferer. Who refers a mint, and gets a cut of the mint fee for that mint.
         * 5. Test, to show that all the fee go to the mint refferer, instead of mint refferer & collection referrer.
         */
        Target[] memory attributions = new Target[](2);
        attributions[0] = Target({chainId: 1, target: address(1)});
        attributions[1] = Target({chainId: 2, target: address(2)});

        address[] memory recipients = new address[](3);
        recipients[0] = address(0xc0ffee);
        recipients[1] = attributions[0].target;
        recipients[2] = attributions[1].target;

        uint256[] memory allocations = new uint256[](3);
        allocations[0] = 750_000;
        allocations[1] = 125_000;
        allocations[2] = 125_000;

        MockSplitFactory splitFactory =
            MockSplitFactory(payable(address(feeManager.splitFactory())));
        vm.expectCall(
            address(splitFactory),
            abi.encodeCall(
                splitFactory.createSplit,
                (
                    SplitV2Lib.Split({
                        recipients: recipients,
                        allocations: allocations,
                        totalAllocation: 1e6,
                        distributionIncentive: 0
                    }),
                    address(feeManager),
                    address(0xc0ffee)
                )
            )
        );

        Target memory receiver = feeManager.createRoute(IEdition(address(mockEdition)), 1, attributions, address(7777));
        assertEq(receiver.target, address(feeManager.splitFactory()));
        assertEq(feeManager.feeReceiver(IEdition(address(mockEdition)), 1).target, receiver.target);
        assertEq(feeManager.referrers(IEdition(address(mockEdition))), address(7777));

        address Edition = feeManager.referrers(IEdition(address(mockEdition)));
        assertEq(feeManager.referrers(IEdition(address(mockEdition))), address(7777));

        address _refferer = address(1);

        feeManager.collectMintFee{value: 1 ether}( IEdition(address(mockEdition)), 1, 1, address(this), address(_refferer));

        console.log("The _refferer balance after the fee collection,", address(_refferer).balance);
        console.log("The edition refferer balance after the fee collection", address(7777).balance);
    }

Impact

The collectionReferrerShare will not receive the fees, while the mintReferrer(_referrer) could receive x2 fees. Note that _referrer could be any arbitrary address.

Code Snippet

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

Tool used

Manual Review

Recommendation

When you route the collectionReferrerShare use the referrers[edition] instead of _referrer.

Duplicate of #267

ShaheenRehman commented 4 months ago

Escalate

This finding is a valid dup of #267

sherlock-admin3 commented 4 months ago

Escalate

This finding is a valid dup of #267

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.

WangSecurity commented 4 months ago

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

Evert0x commented 4 months ago

Result: High Duplicate of #267

sherlock-admin4 commented 4 months ago

Escalations have been resolved successfully!

Escalation status: