highest bidder is able to cancle his bid using _cancelAllBids function

Summary

In the following code is stated that the highest bidder should not be able to cancel his bid if it is the highest one for the auction:

/**
     * @notice Cancel bid for current round if not highest bidder
     */
    function _cancelBid(
        uint256 tokenId,
        uint256 round,
        address bidder
    ) internal {
        EnglishPeriodicAuctionStorage.Layout
            storage l = EnglishPeriodicAuctionStorage.layout();

        address currentBidder;
        if (IStewardLicense(address(this)).exists(tokenId)) {
            currentBidder = IStewardLicense(address(this)).ownerOf(tokenId);
        } else {
            currentBidder = l.initialBidder;
        }

        require(
            bidder != l.highestBids[tokenId][round].bidder,
            'EnglishPeriodicAuction: Cannot cancel bid if highest bidder'
        );

        Bid storage bid = l.bids[tokenId][round][bidder];

        require(
            bid.collateralAmount > 0,
            'EnglishPeriodicAuction: No bid to cancel'
        );

        // Make collateral available to withdraw
        l.availableCollateral[bidder] += bid.collateralAmount;

        // Reset collateral and bid
        bid.collateralAmount = 0;
        bid.bidAmount = 0;
    }

An user is able to cancel his bid and bypass the require check if he uses _cancelAllBids function:

/**
     * @notice Cancel bids for all rounds
     */
    function _cancelAllBids(uint256 tokenId, address bidder) internal {
        EnglishPeriodicAuctionStorage.Layout
            storage l = EnglishPeriodicAuctionStorage.layout();

        uint256 currentAuctionRound = l.currentAuctionRound[tokenId];

        for (uint256 i = 0; i <= currentAuctionRound; i++) {
            Bid storage bid = l.bids[tokenId][i][bidder];

            if (bid.collateralAmount > 0) {
                // Make collateral available to withdraw
                l.availableCollateral[bidder] += bid.collateralAmount;

                // Reset collateral and bid
                bid.collateralAmount = 0;
                bid.bidAmount = 0;
            }
        }
    }

Vulnerability Detail

The vulnerability lies in the inconsistency between the _cancelBid and _cancelAllBids functions within the contract. While the _cancelBid function correctly checks whether the bidder is the highest bidder for the auction before allowing bid cancellation, the _cancelAllBids function lacks this crucial check. Consequently, a bidder can exploit this discrepancy to cancel their bid even if it happens to be the highest bid for the auction.

Impact

Bidder can cancel his bid even if it is the highest for the auction

Code Snippet

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

Tool used

Manual Review

Recommendation

Implement a require statement in _cancelAllBids like it is used in _cancelBid

Duplicate of #14

sherlock-audit / 2024-02-radicalxchange-judging

valentin2304 - highest bidder is able to cancle his bid using _cancelAllBids function #81