KiroBrejka - [M-1] - Seller's `quoteToken` may remain locked in the protocol under certain conditions

[sherlock-admin3]

KiroBrejka

high

[M-1] - Seller's quoteToken may remain locked in the protocol under certain conditions

Summary

Seller's funds may remain locked in the protocol, because of revert on 0 transfer tokens. In the README.md file is stated that the protocol uses every token with ERC20 Metadata and decimals between 6-18, which includes some revert on 0 transfer tokens, so this should be considered as valid issue!

Vulnerability Detail

in the AuctionHouse::claimProceeds() function there is the following block of code:

       uint96 prefundingRefund = routing.funding + payoutSent_ - sold_;
        unchecked {
            routing.funding -= prefundingRefund;
        }
        Transfer.transfer(
            routing.baseToken,
            _getAddressGivenCallbackBaseTokenFlag(routing.callbacks, routing.seller),
            prefundingRefund,
            false
        );

Since the batch auctions must be prefunded so routing.funding shouldn’t be zero unless all the tokens were sent in settle, in which case payoutSent will equal sold_. From this we make the conclusion that it is possible for prefundingRefund to be equal to 0. This means if the routing.baseToken is a revert on 0 transfer token the seller will never be able to get the quoteToken he should get from the auction.

Impact

The seller's funds remain locked in the system and he will never be able to get them back.

Code Snippet

The problematic block of code in the AuctionHouse::claimProceeds() function: https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/AuctionHouse.sol#L604-L613

Transfer::transfer() function, since it transfers the baseToken: https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/lib/Transfer.sol#L49-L68

Tool used

Manual Review

Recommendation

Check if the prefundingRefund > 0 like this:

   function claimProceeds(
        uint96 lotId_,
        bytes calldata callbackData_
    ) external override nonReentrant {
        // Validation
        _isLotValid(lotId_);

        // Call auction module to validate and update data
        (uint96 purchased_, uint96 sold_, uint96 payoutSent_) =
            _getModuleForId(lotId_).claimProceeds(lotId_);

        // Load data for the lot
        Routing storage routing = lotRouting[lotId_];

        // Calculate the referrer and protocol fees for the amount in
        // Fees are not allocated until the user claims their payout so that we don't have to iterate through them here
        // If a referrer is not set, that portion of the fee defaults to the protocol
        uint96 totalInLessFees;
        {
            (, uint96 toProtocol) = calculateQuoteFees(
                lotFees[lotId_].protocolFee, lotFees[lotId_].referrerFee, false, purchased_
            );
            unchecked {
                totalInLessFees = purchased_ - toProtocol;
            }
        }

        // Send payment in bulk to the address dictated by the callbacks address
        // If the callbacks contract is configured to receive quote tokens, send the quote tokens to the callbacks contract and call the onClaimProceeds callback
        // If not, send the quote tokens to the seller and call the onClaimProceeds callback
        _sendPayment(routing.seller, totalInLessFees, routing.quoteToken, routing.callbacks);

        // Refund any unused capacity and curator fees to the address dictated by the callbacks address
        // By this stage, a partial payout (if applicable) and curator fees have been paid, leaving only the payout amount (`totalOut`) remaining.
        uint96 prefundingRefund = routing.funding + payoutSent_ - sold_;
++ if(prefundingRefund > 0) { 
        unchecked {
            routing.funding -= prefundingRefund;
        }
            Transfer.transfer(
            routing.baseToken,
            _getAddressGivenCallbackBaseTokenFlag(routing.callbacks, routing.seller),
            prefundingRefund,
            false
        );
++        }

        // Call the onClaimProceeds callback
        Callbacks.onClaimProceeds(
            routing.callbacks, lotId_, totalInLessFees, prefundingRefund, callbackData_
        );
    }

[nevillehuang]

21, #31 and #112 highlights the same issue of prefundingRefund = 0

78 and #97 highlights the same less likely issue of totalInLessFees = 0

All points to same underlying root cause of such tokens not allowing transfer of zero, so duplicating them. Although this involves a specific type of ERC20, the impact could be significant given seller's fund would be locked permanently

[sherlock-admin4]

The protocol team fixed this issue in the following PRs/commits: https://github.com/Axis-Fi/moonraker/pull/142

[10xhash]

The protocol team fixed this issue in the following PRs/commits: Axis-Fi/moonraker#142

Fixed Now Transfer library only transfers token if amount > 0

[sherlock-admin4]

The Lead Senior Watson signed off on the fix.