Highest bidder can cancel his bid even though he's not supposed to

Summary

Highest bidder shouldn't be able to cancel his bid. This is can be seen by this require statement in the _cancelBid function in EnglishPeriodicAuctionInternal:

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

The problem is that this can be circumvented by using the _cancelAllBids since this function doesn't have the same require statement.

Vulnerability Detail

The highest bidder can still cancel his bid even though he's not supposed to be able to do that.

Impact

This will lead to the to 2nd highest bidder winning the auction even though he didn't expect this. This behavior bypasses protocol's intended behavior.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add a check in _cancelAllBids that checks if it's current round and the bidder is the highest bidder, he should not be able to cancel the bid.

Duplicate of #14

sherlock-audit / 2024-02-radicalxchange-judging

ethernal - Highest bidder can cancel his bid even though he's not supposed to #99