auditsbyradev - `NounsAuctionHouseV2.sol#_createAuction()` - Any revert caused by minting will not be captured

[sherlock-admin2]

auditsbyradev

high

NounsAuctionHouseV2.sol#_createAuction() - Any revert caused by minting will not be captured

Summary

The NounsAuctionHouseV2.sol contract is designed to manage the auctioning process for Nouns NFTs. It includes functionality to create and manage auctions, bid on NFTs, settle auctions, and handle the minting of Nouns NFTs upon auction completion. The _createAuction() function within this contract is very important, as it is responsible for initializing a new auction, including the minting of a new Noun NFT to be auctioned.

In the context of NounsAuctionHouseV2.sol, the _createAuction() function is called after an auction is successfully settled to prepare the next auction. This involves minting the next Noun NFT, setting up auction parameters (like start time and end time), and storing this information within the contract's state.

Vulnerability Detail

The try-catch block in the _createAuction() function is designed to catch failures during the NFT minting process. However, the catch block is only configured to catch exceptions thrown as strings (using the Error(string) pattern), which is a limitation. This setup does not catch other types of reverts or custom errors that may be thrown by the NFT minting function (mint()), particularly those not encapsulated as string messages. Any revert caused by minting will not be captured. This means that in the case of a erroneous mint the contract will be effectively bricked (and not paused)

the same bug exists and in NounsAuctionHouse.sol

Impact

Should the mint() function within the _createAuction() process revert due to any reason other than an error thrown as a string message, the auction contract does not pause as it ideally should. This omission leaves the contract in a vulnerable state where subsequent auctions cannot be initiated, effectively rendering the auction process inoperative or "bricked," without an external intervention to pause or reset the contract.

Summary of the Impact:

• In case of an error at the mint() function the auction won't be settled till the owner intervenes and pauses the contract.
• Any revert caused by minting will not be captured. This means that in the case of a erroneous mint the contract will be effectively bricked (and not paused)

Code Snippet

• NounsAuctionHouseV2.sol#_createAuction()

  function _createAuction() internal {
      try nouns.mint() returns (uint256 nounId) {
          uint40 startTime = uint40(block.timestamp);
          uint40 endTime = startTime + uint40(duration);
  
          auctionStorage = AuctionV2({
              nounId: uint96(nounId),
              clientId: 0,
              amount: 0,
              startTime: startTime,
              endTime: endTime,
              bidder: payable(0),
              settled: false
          });
  
          emit AuctionCreated(nounId, startTime, endTime);
      } catch Error(string memory) {
          _pause();
      }
  }

Tool used

Manual Review

Recommendation

Adjust the catch block to capture all reverts, not just those that emit a string error.

    function _createAuction() internal {
        try nouns.mint() returns (uint256 nounId) {
            uint40 startTime = uint40(block.timestamp);
            uint40 endTime = startTime + uint40(duration);

            auctionStorage = AuctionV2({
                nounId: uint96(nounId),
                clientId: 0,
                amount: 0,
                startTime: startTime,
                endTime: endTime,
                bidder: payable(0),
                settled: false
            });

            emit AuctionCreated(nounId, startTime, endTime);
-       } catch Error(string memory) {
+       } catch {
            _pause();
        }
    }

This way regardless of the nature of the error encountered during the minting process, the contract will be paused, allowing for manual intervention and resolution, thereby preventing the contract from becoming bricked and maintaining the integrity of the auction process.

[sherlock-admin3]

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

WangAudit commented:

what specific reverts can be missed? what reverts are not in strings? I believe this support is not sufficient enough to be validated since it addresses a pontential problem without any details (i.e. specific reverts that wouldn't be catched)