auditsbyradev - `NounsAuctionHouseV2.sol` - Auction parameters are not individually cached for each auction

[sherlock-admin2]

auditsbyradev

high

NounsAuctionHouseV2.sol - Auction parameters are not individually cached for each auction

Summary

The NounsAuctionHouseV2.sol contract is designed to facilitate the auctioning of Nouns NFTs, playing a central role in the Nouns DAO ecosystem. This contract manages the entire lifecycle of an auction, from creation, bidding, to settlement. Each auction involves minting a new Noun NFT, setting it up for auction, accepting bids, and ultimately transferring the NFT to the highest bidder, with the process designed to be transparent and fair to all participants.

Auction Process

1. Auction Creation: Initiated by the contract, often following the settlement of the previous auction. It involves minting the NFT and setting the auction parameters.
2. Bidding: Participants can place bids on the NFT, with each bid needing to be higher than the last by a specified minimum increment.
3. Settlement: Upon auction completion, the highest bidder wins the NFT. The contract then prepares for the next auction, repeating the cycle.

Vulnerability Detail

The problem in NounsAuctionHouseV2.sol contract is that the auction parameters, such as the minimum bid increment, reserve price, and time buffer, are managed as global settings rather than being cached individually for each auction. This means that changes to these parameters affect all ongoing and future auctions.

Global Parameters: In the current implementation, the contract uses single, global state variables for timeBuffer, reservePrice, and minBidIncrementPercentage. These variables apply to the active auction and any future auctions until they are changed.

Lack of Individual Auction Context: Each auction does not have its own set of these parameters. Consequently, if an auction is ongoing and one of these parameters is changed, the new values immediately affect the current auction.

Potential Exploitation: If these parameters are changed while an auction is active. This can lead to scenarios where the rules of the auction are altered in real-time, potentially disadvantaging certain bidders or favoring others. For example, increasing the minBidIncrementPercentage could unexpectedly raise the minimum additional bid amount required, while decreasing the timeBuffer could shorten the auction duration unexpectedly.

Impact

This design choice can lead to inconsistencies and potentially unfair advantages if the parameters are changed during an ongoing auction. For example:

• some sudden changes may cause bidder's transaction fail, such as setReservePrice() and setMinBidIncrementPercentage()
  • Bidder Disadvantage: A sudden increase in the minimum bid increment could prevent bidders from participating if their intended bids no longer meet the new minimum requirement.
• some changes may change users expectation about the auction, such as setTimeBuffer(), with different time parameters, bidders will use different strategy
  • Unexpected Auction Closures: Modifying the time buffer could abruptly shorten or extend an auction, catching bidders off-guard.

Code Snippet

NounsAuctionHouseV2.sol#setTimeBuffer()

    /**
     * @notice Set the auction time buffer.
     * @dev Only callable by the owner.
     */
    function setTimeBuffer(uint56 _timeBuffer) external override onlyOwner {
        require(_timeBuffer <= MAX_TIME_BUFFER, 'timeBuffer too large');

        timeBuffer = _timeBuffer;

        emit AuctionTimeBufferUpdated(_timeBuffer);
    }

NounsAuctionHouseV2.sol#setReservePrice()

    /**
     * @notice Set the auction reserve price.
     * @dev Only callable by the owner.
     */
    function setReservePrice(uint192 _reservePrice) external override onlyOwner {
        reservePrice = _reservePrice;

        emit AuctionReservePriceUpdated(_reservePrice);
    }

NounsAuctionHouseV2.sol#setMinBidIncrementPercentage()

    /**
     * @notice Set the auction minimum bid increment percentage.
     * @dev Only callable by the owner.
     */
    function setMinBidIncrementPercentage(uint8 _minBidIncrementPercentage) external override onlyOwner {
        require(_minBidIncrementPercentage > 0, 'must be greater than zero');

        minBidIncrementPercentage = _minBidIncrementPercentage;

        emit AuctionMinBidIncrementPercentageUpdated(_minBidIncrementPercentage);
    }

Tool used

Manual Review

Recommendation

Implement a mechanism where each auction caches its own set of parameters at the time of creation. This ensures that the rules of an auction remain consistent throughout its duration. Another possible solution is to add a whenPaused modifier in the setter functions of auction parameters so that the auction parameters cannot be changed while there is an active auction.

[sherlock-admin2]

2 comment(s) were left on this issue during the judging contest.

WangAudit commented:

I believe it's intended behaviour and admins will only make reasonable changes at a reasonable time

takarez commented:

this is invalid as the admin has the responsibility to change this parameters.