Al-Qa-qa - Unbidded Auctions can not get closed because of not checking `repossessor` address.

[sherlock-admin4]

Al-Qa-qa

medium

Unbidded Auctions can not get closed because of not checking repossessor address.

Summary

Because of not checking the repossessor address when initializing the collection, or setting it, the Auction will be Unclosable, and The token can get locked in the contract.

Vulnerability Detail

If no one bids in the current round of the auction, the token is transferred to the repossessor address, but if this address can not receive the token, the auction can not be closed.

auction/EnglishPeriodicAuctionInternal.sol#L526-L530

    function _closeAuction(uint256 tokenId) internal {
        ...

        if (l.highestBids[tokenId][currentAuctionRound].bidder == address(0)) {
            // No bids were placed, transfer to repossessor
            Bid storage repossessorBid = l.bids[tokenId][currentAuctionRound][
                l.repossessor
            ];
            repossessorBid.bidAmount = 0;
            repossessorBid.feeAmount = 0;
            repossessorBid.collateralAmount = 0;
            repossessorBid.bidder = l.repossessor;

            // @audit set the Highest Bidder to `repossessor`, if no one Bids
<@          l.highestBids[tokenId][currentAuctionRound] = repossessorBid;
        } else if (
            l.highestBids[tokenId][currentAuctionRound].bidder != oldBidder
        ) { ... } else { ... }

        ...

        // Transfer to highest bidder
        IStewardLicense(address(this)).triggerTransfer(
            oldBidder,
<@          l.highestBids[tokenId][currentAuctionRound].bidder,
            tokenId
        );

        ...
    }

This function either transfers the token or mints it if it does not exist.

license/StewardLicenseInternal.sol#L96-L108

    function _triggerTransfer( ... ) internal {
        if (!_exists(tokenId)) {
            // Mint token
            _mint(to, tokenId);
        } else {
            _transfer(from, to, tokenId);
        }
    }

The problem is that if the receiver of the token is address(0), mint/transfer the ERC721 token will revert.

When setting the repossessor address, the function does not check if the address provided is address(0) or not.

auction/EnglishPeriodicAuctionInternal.sol#L88-L92

    // @audit Missing address(0) check
    function _setRepossessor(address repossessor) internal {
        EnglishPeriodicAuctionStorage.layout().repossessor = repossessor;

        emit RepossessorSet(repossessor);
    }

Missing address(0) check is categorized as a LOW issue, But in this case, the Impact is HIGH. where this will lead to preventing the token from completing its Period Auction cycle, which is the desire of the protocol.

And if the Collection is deployed without an owner, No one will be able to change the repossessor address.

Impact

• Token getting locked in the Contract, if it was minted.
• The periodic Auction Cycle Mechanism will stop for that token, and there will no be any future Auction rounds.

Code Snippet

https://github.com/sherlock-audit/2024-02-radicalxchange/blob/main/pco-art/contracts/auction/EnglishPeriodicAuctionInternal.sol#L88-L92

Tool used

Manual Review

Recommendation

Check that the provided address is not address(0).

EnglishPeriodicAuctionInternal::_setRepossessor()

function _setRepossessor(address repossessor) internal {
+       require(repossessor != address(0), "Zero address is not allowed")
EnglishPeriodicAuctionStorage.layout().repossessor = repossessor;

    emit RepossessorSet(repossessor);
}