sherlock-audit / 2024-04-titles-judging

9 stars 6 forks source link

popeye - Incorrect Fee Distribution in `FeeManager.sol` Contract Affects Referrer Rewards #176

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

popeye

high

Incorrect Fee Distribution in FeeManager.sol Contract Affects Referrer Rewards

Summary

The _splitProtocolFee function in the FeeManager.sol contract incorrectly distributes the collection referrer's share of the protocol fee to the mint referrer instead of the actual collection referrer. This bug affects the fair distribution of referrer shares and can lead to incorrect accounting and lack of incentive for collection referrers.

Vulnerability Detail

The _splitProtocolFee function is responsible for splitting the protocol fee among the protocol fee receiver, mint referrer, and collection referrer. However, there is an issue in the way the collection referrer's share is being distributed.

In the current implementation, the collection referrer's share (collectionReferrerShare) is being sent to the referrer_ address, which represents the mint referrer. This means that the mint referrer is receiving both their own share (mintReferrerShare) and the collection referrer's share, effectively doubling their intended share. Meanwhile, the actual collection referrer, whose address should be referrers[edition_], is not receiving any share at all.

Here's the affected code snippet:

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

    //..........other logics...

   _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}), //@audit-issue - here should be target: referrers[edition_] instead of just referrer_
       payer_
   );

As shown above, both the mintReferrerShare and collectionReferrerShare are being sent to the referrer_ address, which is incorrect for the collection referrer's share. This bug leads to an unfair distribution of rewards, as the mint referrer receives more than their fair share, while the collection referrer receives nothing. It can also result in incorrect accounting and reporting of referrer shares, causing discrepancies in financial records.

Impact

The incorrect distribution of the collection referrer's share has several impacts:

Code Snippet

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

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, the code in the _splitProtocolFee function should be updated to correctly send the collection referrer's share to their address instead of the mint referrer. Here's the recommended code change:

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

    //..........other logics...

   _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}),
+       Fee({asset: asset_, amount: collectionReferrerShare}),
+       Target({target: referrers[edition_], chainId: block.chainid}),
       payer_
   );

Duplicate of #267

sammy-tm commented 4 months ago

Escalate

Dup of #267

sherlock-admin3 commented 4 months ago

Escalate

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: