Flawed bid cancellation logic allows user to win auction with 100% certainty without even spending any money

Summary

The logic to cancel all bids differs from cancelling individual bids which allows a user to bid max amount and guarantee auction win, while also refunding their bid

Vulnerability Detail

If we try to cancel a bid while we are the highest bidder, the following line prohibits us from doing so:

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

The issue is that the same line is missing in the function to cancel all bids, while it also includes and cancels the bid for the current round in the for-loop:

    for (uint256 i = 0; i <= currentAuctionRound; i++) { // @audit i <= currentAuctionRound instead of <

The cancel all bids function changes the bid in storage, but does not adjust the highest bid in storage.

This allows for the following scenario:

User bids type(uint256).max on an auction
User calls _cancelAllBids()
His bid is cancelled and added back to the l.availableCollateral[bidder]
The highest bid in storage remains untouched (bid is type(uint256).max)
User withdraws collateral

Now no one can out-bid the user since they cannot bid more than the max value for the uint, the auction concludes and the user wins it while also having already withdrawn his bid/collateral.

Impact

User can bid max uint value, wins auction with 100% certainty AND doesn't even spend anything since he withdrew his collateral. Free auction win, 100% certainty. Logic is completely flawed.

Code Snippet

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

Tool used

Manual Review

Recommendation

Either change for-loop to < instead of <= or make sure user is not highest bidder like in singular cancels.

Duplicate of #14

sherlock-audit / 2024-02-radicalxchange-judging

cats - Flawed bid cancellation logic allows user to win auction with 100% certainty without even spending any money #18