cancelAllBids does not check if bid is highest bid.

Summary

user can withdraw the funds from their highest bid

Vulnerability Detail

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

Users can at any time invoke cancelAllBids and withdraw all their funds from bids on a chosen token. The problem is that the function does not check if the bid is actually the highest one for that said token.

This would allow any user to make a high bid and withdraw it, allowing them to win an auction without actually spending funds on it.

Impact

Theft of NFT

Code Snippet

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

Tool used

Manual Review

Recommendation

check if user bid is highest bid

Duplicate of #14

sherlock-audit / 2024-02-radicalxchange-judging

koreanspicygarlic - cancelAllBids does not check if bid is highest bid. #104