auditsbyradev - `NounsAuctionHouseV2.sol` - If `reservedPrice` is changed through an active auction, the auction still can be settled. This will result in losses for the NFT owner or auction bidder

[sherlock-admin4]

auditsbyradev

high

NounsAuctionHouseV2.sol - If reservedPrice is changed through an active auction, the auction still can be settled. This will result in losses for the NFT owner or auction bidder

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 is that the reservedPrice parameter can be altered during an active auction. Since reservedPrice directly influences the validity of bids and the auction's outcome, changes to this parameter mid-auction can lead to unintended consequences. If the reservedPrice is increased, the NFT owner risks the auction settling below this new threshold, incurring a loss. Conversely, lowering the reservedPrice could lead to bidders overpaying based on the initial auction conditions.

Impact

• Owner Losses: Increasing the reservedPrice mid-auction can result in the auction settling at a price below the newly set reserve, leading to potential losses for the NFT owner.
• Bidder Overpayment: Decreasing the reservedPrice during an auction can cause bidders to pay more than the updated reserve, potentially leading to bidder dissatisfaction and trust issues in the auction process.

Code Snippet

• NounsAuctionHouseV2.sol#_settleAuction()

  /**
   * @notice Settle an auction, finalizing the bid and paying out to the owner.
   * @dev If there are no bids, the Noun is burned.
   */
  function _settleAuction() internal {
      INounsAuctionHouseV2.AuctionV2 memory _auction = auctionStorage;
  
      require(_auction.startTime != 0, "Auction hasn't begun");
      require(!_auction.settled, 'Auction has already been settled');
      require(block.timestamp >= _auction.endTime, "Auction hasn't completed");
  
      auctionStorage.settled = true;
  
      if (_auction.bidder == address(0)) {
          nouns.burn(_auction.nounId);
      } else {
          nouns.transferFrom(address(this), _auction.bidder, _auction.nounId);
      }
  
      if (_auction.amount > 0) {
          _safeTransferETHWithFallback(owner(), _auction.amount);
      }
  
      SettlementState storage settlementState = settlementHistory[_auction.nounId];
      settlementState.blockTimestamp = uint32(block.timestamp);
      settlementState.amount = ethPriceToUint64(_auction.amount);
      settlementState.winner = _auction.bidder;
      if (_auction.clientId > 0) settlementState.clientId = _auction.clientId;
  
      emit AuctionSettled(_auction.nounId, _auction.bidder, _auction.amount);
      if (_auction.clientId > 0) emit AuctionSettledWithClientId(_auction.nounId, _auction.clientId);
  }

• 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);
  }

Tool used

Manual Review

Recommendation

possible solution is to add a whenPaused modifier in the setReservePrice() function so that the reservePrice cannot be changed while there is an active auction.

[sherlock-admin3]

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

WangAudit commented:

assume owners are trusted not to this and it at max low/info

takarez commented:

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

[radeveth]

I believe my bug is valid because it highlights a fundamental risk in the NounsAuctionHouseV2.sol contract, irrespective of the level of trust placed in the administrators. The ability to alter the reservedPrice mid-auction directly conflicts with the principles of transparency and fairness that are crucial to the auction process. This vulnerability can lead to tangible financial losses for NFT owners or cause bidders to overpay, undermining confidence in the Nouns DAO ecosystem. Trust in administrators does not negate the potential for human error or the unforeseen need to adjust auction parameters in response to dynamic market conditions. Furthermore, relying solely on administrative discretion without implementing safeguards diminishes the decentralized ethos of blockchain applications, where smart contract mechanisms are designed to provide trustless interactions. Implementing a whenPaused modifier on the setReservePrice() function is a practical and necessary step to prevent unintended consequences and ensure that auction integrity is maintained, regardless of administrator actions.

Finally, by the judge's comments, you are telling me that the reserved price will only change if there is no active auction (otherwise the bug exists)? The bug is not related to a trusted owner or anything like that!

Also for reference I will provide a Nouns-like DAO protocol (turnover protocol) that implements logic that prevents the auction from being settled if the reservedPrice changes through this active auction.

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/main/packages/revolution/src/AuctionHouse.sol#L347-L356

[radeveth]

The problem is that the reservedPrice parameter can be altered during an active auction. Since reservedPrice directly influences the validity of bids and the auction's outcome, changes to this parameter mid-auction can lead to unintended consequences. If the reservedPrice is increased, the NFT owner risks the auction settling below this new threshold, incurring a loss. Conversely, lowering the reservedPrice could lead to bidders overpaying based on the initial auction conditions.

• Owner Losses: Increasing the reservedPrice mid-auction can result in the auction settling at a price below the newly set reserve, leading to potential losses for the NFT owner.
• Bidder Overpayment: Decreasing the reservedPrice during an auction can cause bidders to pay more than the updated reserve, potentially leading to bidder dissatisfaction and trust issues in the auction process.

[WangSecurity]

Firstly, the problem is that report can be viable only if the owner makes a mistake or acts maliciously, therefore, I think owners are trusted not to do so. I understand that in edge case it can happen, but this is only informational finding or low severity at max, cause it relies on owner to make a mistake.

Another problem, when settling the reserve price mid-auction, it doesn't effect already created bids or settling of the auction. reservePrice, i.e. the check that user's bid is higher than reservePrice, is only checked during creation of the bid.

Therefore, if there is current reserve price is 0.01ETH, current bid is 0.02 ETH and owner changes reservePrice to 0.03 ETH, the auction will still settle without any issue and the bidder will bet his NFT. And vice versa, I don't think "overpaying" is sufficient enough to be medium scenario in that case, cause as I see on NounsDAO site, their auctions always end up much much higher than the reservePrice. Yes, it's not a very strong argument, but what I mean here is that in case of "overpaying" both likelihood and impact are extremely low.