Protocol Fee Changes Impacting Auction Settlements

Summary

The AuctionHouse contract is vulnerable to unexpected financial outcomes due to protocol fee changes during an active auction. Sellers and buyers could receive different amounts than anticipated if fees are altered before the auction's conclusion.

Vulnerability Detail

Current Implementation: The contract calculates fees during the settle function based on the current fee settings.

What Goes Wrong:

If protocol fees are updated after bids are placed but before the auction is settled, the final distribution of funds can differ from the expectations at the time of bidding.

// In the AuctionHouse contract
function settle(uint96 lotId_) external override nonReentrant {
...
// @audit Fees are calculated here based on the current settings
uint96 totalInLessFees;
{
    (, uint96 toProtocol) = calculateQuoteFees(
        lotFees[lotId_].protocolFee, lotFees[lotId_].referrerFee, false, purchased_
    );
     unchecked {
    totalInLessFees = purchased_ - toProtocol;
 }
}
...
}

Impact

leading to a potential loss of trust in the platform as users may receive less than expected due to fee changes.

Code Snippet

given above. https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/AuctionHouse.sol#L587

Tool used

Manual Review

Recommendation

To mitigate this issue, protocol fees should be locked in at the time of bid placement or auction creation. This ensures consistency and predictability of fees for all parties involved.
When a bid is placed or an auction is created, the current protocol fee should be recorded and associated with that specific auction or bid.
During settlement, the contract should use the recorded fee rather than the potentially updated global fee setting.

Implementation: The purchase function already calculates fees at the time of the transaction, which is a one-time operation and not subject to changes in fees during an auction's lifecycle. This immediate fee deduction upon purchase should be mirrored in the auction settlement process to ensure consistency.

nevillehuang commented 7 months ago

Invalid, admin action, invalid based on the following sherlock rule

An admin action can break certain assumptions about the functioning of the code. Example: Pausing a collateral causes some users to be unfairly liquidated or any other action causing loss of funds. This is not considered a valid issue.

sherlock-admin4 commented 7 months ago

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

10xhash commented 7 months ago

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

Fixed The fees are cached at the time of auction creation