cryptonoob - Token buyers can pay less fees in AuctionHouse::purchase setting the referer as himself

[sherlock-admin4]

cryptonoob

medium

Token buyers can pay less fees in AuctionHouse::purchase setting the referer as himself

Summary

AuctionHouse::purchase function allows token buyers to pay less fess while purchasing tokens as it allows setting himself as referer thus substracting the total fee

This means that if a user set himself as the referer then he will gain the referrer fee thus paying less fees to the protocol (because protocolfee = protocolfee - referrerFee )

Vulnerability Detail

The vulnerability exists in AuctionHouse::purchase because it allows setting referrer as caller ie msg.sender

function purchase(
        PurchaseParams memory params_,
        bytes calldata callbackData_
    ) external override nonReentrant returns (uint96 payoutAmount) {
        // ... snippet
        // Calculate quote fees for purchase
        uint96 amountLessFees;
        {
            Keycode auctionKeycode = keycodeFromVeecode(routing.auctionReference);
            //... snippet
            uint96 totalFees = _allocateQuoteFees(
                fees[auctionKeycode].protocol,
                fees[auctionKeycode].referrer,
                params_.referrer,                   //<@==== user provided referrer
                routing.seller,
                routing.quoteToken,
                params_.amount
            );
            unchecked {
                amountLessFees = params_.amount - totalFees;
            }
        }

The referrer is set in PurchaseParams params_ argument and then allocateQuoteFees is called:

    function _allocateQuoteFees(
        uint96 protocolFee_,
        uint96 referrerFee_,
        address referrer_,              //<@==== user provided referrer
        //... snippet
    ) internal returns (uint96 totalFees) {
        // Calculate fees for purchase
        (uint96 toReferrer, uint96 toProtocol) = calculateQuoteFees(
            protocolFee_, referrerFee_, referrer_ != address(0) && referrer_ != seller_, amount_
        );
        //... snippet
    }

That calls FeeManager::calculateQuoteFees:

    function calculateQuoteFees(
        uint96 protocolFee_,
        uint96 referrerFee_,
        bool hasReferrer_,
        uint96 amount_
    ) public pure returns (uint96 toReferrer, uint96 toProtocol) {
        uint96 feeDecimals = uint96(_FEE_DECIMALS);
        if (hasReferrer_) {
            // In this case we need to:
            // 1. Calculate referrer fee
            // 2. Calculate protocol fee as the total expected fee amount minus the referrer fee
            //    to avoid issues with rounding from separate fee calculations
            toReferrer = uint96(Math.mulDivDown(amount_, referrerFee_, feeDecimals)); // <@ referrer
            toProtocol = uint96(Math.mulDivDown(amount_, protocolFee_ + referrerFee_, feeDecimals))
                - toReferrer;
        } //... snippet 
    }

As is seen the referer Fee is substracted from protocolFee, so if the user set himself as the referer then he will pay less to purchase tokens. To show it, create a new testcase in test/AuctionHouse/purchase.t.sol

function test_givenReferrerFeeIsSetSelf()
    external
        whenAuctionTypeIsAtomic
        whenAtomicAuctionModuleIsInstalled
        givenLotIsCreated
        givenReferrerFeeIsSet
        givenProtocolFeeIsSet
        givenLotHasStarted
        givenUserHasQuoteTokenBalance(_AMOUNT_IN)
        givenUserHasQuoteTokenAllowance(_AMOUNT_IN)
        givenFeesAreCalculated(_AMOUNT_IN)
        whenPayoutMultiplierIsSet(_PAYOUT_MULTIPLIER)
        givenBalancesAreCalculated(_AMOUNT_IN, _amountOut)
        givenSellerHasBaseTokenBalance(_amountOut)
        givenSellerHasBaseTokenAllowance(_amountOut)
    {
        // Purchase
        _createPurchaseSelf(_AMOUNT_IN, _amountOut, _purchaseAuctionData, address(this));

        // Check state
        _assertQuoteTokenBalances();
        _assertBaseTokenBalances();
        _assertDerivativeTokenBalances();
        assertEq(
            _auctionHouse.rewards(address(this), _quoteToken),
            _expectedReferrerFeesAllocated,
            "referrer fee"
        );
        _assertPrefunding();
    }

And add this function in test/AuctionHouse/AuctionHouseTest.sol

function _createPurchaseSelf(
        uint96 amount_,
        uint96 minAmountOut_,
        bytes memory auctionData_,
        address referrer_
    ) internal returns (uint256) {
        Router.PurchaseParams memory purchaseParams = Router.PurchaseParams({
            recipient: msg.sender,
            referrer: msg.sender,
            lotId: _lotId,
            amount: amount_,
            minAmountOut: minAmountOut_,
            auctionData: auctionData_,
            permit2Data: _permit2Data
        });

        vm.prank(msg.sender);
        uint256 payout = _auctionHouse.purchase(purchaseParams, _allowlistProof);

        return payout;
    }

Impact

The impact of this vulnerability includes:

1. Paying less fees to the protocol while purchasing tokens

Code Snippet

https://github.com/sherlock-audit/2024-03-axis-finance/blob/cadf331f12b485bac184111cdc9ba1344d9fbf01/moonraker/src/AuctionHouse.sol#L201-L217
https://github.com/sherlock-audit/2024-03-axis-finance/blob/cadf331f12b485bac184111cdc9ba1344d9fbf01/moonraker/src/AuctionHouse.sol#L835-L850
https://github.com/sherlock-audit/2024-03-axis-finance/blob/cadf331f12b485bac184111cdc9ba1344d9fbf01/moonraker/src/bases/FeeManager.sol#L68-L83

Tool used

Manual Review

Recommendation

To mitigate this vulnerability it is recommended to check referer value is not msg.sender in AuctionHouse::purchase params_ argument

function purchase(
        PurchaseParams memory params_,
        bytes calldata callbackData_
    ) external override nonReentrant returns (uint96 payoutAmount) {
        require(params_.referrer != msg.sender , "referer must not be msg.sender");  //<@ added

Duplicate of #133